Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Repeated Arguments using standard syntax instead of the non-standard and confusing syntax. #357

Closed
duaneking opened this issue Oct 31, 2018 · 14 comments

Comments

@duaneking
Copy link

Using the newest version, Despite the fact I declared the option as a list in my options class, I'm not able to add multiple options when I use the standard linux command argument patterns, but I am if I use a pattern that breaks compatibility with some accessibility software used by the disabled.

For example the following creates a repeatedoption error:

    public class CommandLineOptions
    {
        [Option('c', "contains", Required = false, Min = 1, Max = 255)]
        public IList<string> Contains { get; set; }
    }

And the command line:

derp.exe  -c "foo" -c "bar"

..Results in an error.

But if I look at the code here on github (because this isn't documented anywhere I can find, at all, despite being a critical use case) I can get it to work if I do:

derp.exe  -c "foo" "bar"

Respectfully, my first thought on seeing this was "This is a critical defect that explicitly neuters the potential of this library".

My personal usage goal is in the style of core linux tools like grep, awk, sed, etc. So in my mind this should work exactly like the standard linux flow, where either the last argument wins in the case of only needing one on the command, or in the case of expecting a possible list due to duplicate use of an option, a list should be built up in the order the command line options are used.

Now we can clearly see that this does work, its just using a very inconsistent syntax that doesn't take into account the needs of screenreaders (So this library technically discriminates against the disabled. Did you know this?) or consistent linux users - look at the standard grep tool, you can use the same argument multiple times and it just works - but only after looking at the source code could I hunt this down because I could not find any data on repeated options or this confusing and inconsistent syntax anywhere in the docs.

Looking at the code, the tests explicitly do not support the industry standard syntax. This is even more frustrating, because its effectively giving the finger to people who want to support accessibility tools that DEPEND on this classic, standard, "last one wins" argument parsing.

This lack of support makes the library unusable for most of the projects I had planned to use it in, and while that hurts me personally, I'm actually more worried about all the people who cant use software that uses this library for argument parsing because of the nonstandard syntax due to a disability that chains them to accessability tools that by default wont understand this non-standard syntax.

So respectfully, I feel like you are shooting yourself in the foot not supporting this, and I would formally like to ask that you support the standard syntax so that more people can actually use this.

@duaneking duaneking changed the title Support Repeated Arguments using standard syntax instead of the non-standard and confusing comma delimited syntax. Support Repeated Arguments using standard syntax instead of the non-standard and confusing syntax. Oct 31, 2018
@duaneking
Copy link
Author

Updated the title as it was from a prior iteration of what I wrote from when I was testing this to make sure I had all the facts.

@nemec
Copy link
Contributor

nemec commented Oct 31, 2018

accessibility tools that DEPEND on this classic, standard, "last one wins" argument parsing.

Hi @duaneking, can you provide some more info on this? Searches on a variation of "screen reader argument parsing" don't really turn up much.

@duaneking
Copy link
Author

Respectfully, that's not the point as chasing a specific application devalues the consistency argument; The golden rule of thumb on argument parsing is the standard "getopt" pattern/toolset. Its the standard for a reason. See http://man7.org/linux/man-pages/man3/getopt.3.html for more details.

@nemec
Copy link
Contributor

nemec commented Oct 31, 2018

Thanks, I'm asking specifically about "last argument wins" vs. "error on duplicated argument". getopt doesn't really prescribe one or the other, as it's left to the implementer how the program behaves when an argument is seen twice.

@duaneking
Copy link
Author

In Linux/Posix systems the universal rule I have seen used in the past 20 or so years of my experience is that if a parameter argument is only required once and is used twice, the second usage overwrites the first one and thus the first argument is simply parsed, validated, and then ignored.

@ericnewton76
Copy link
Member

The library's intention certainly is to follow getopt() semantics, as @nemec mentioned its not set in stone according to getopt on this issue though.

@duaneking can you provide a few examples you're talking about? This will help nail down an implementation.

Maybe a separate branch for this could be started to address these concerns, make changes to the code base to support it, and fix the tests that make incorrect assumptions

Makes sense that derp -c "1" --asdf -c "2" with a public string C; property would end up having "2" as its value

@moh-hassan
Copy link
Collaborator

moh-hassan commented Nov 2, 2018

I will try match the implementation of CommandLineParser library with the standard of getopt of GNU and POSIX specifications as described in linuxfoundation.org
and also described by
The Open Group Base Specifications Issue 7, 2018 edition
IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008)

Option Characteristics
GNU specifies that:

  • an element of argv that starts with "-" (and is not exactly "-" or "--") is an option element. YES
  • characters of an option element, aside from the initial "-", are option characters. YES

POSIX specifies that:

applications using getopt must obey the following syntax guidelines:

  • option name is a single alphanumeric character from the portable character set YES
  • option is preceded by the "-" delimiter character YES
  • options without option-arguments should be accepted when grouped behind one "-" delimiter YES
  • each option and option-argument is a separate argument YES
  • option-arguments are not optional (Yes, controlled with Required)
  • all options should precede operands on the command line YES
  • the argument "--" is accepted as a delimiter indicating the end of options and the consideration of subsequent arguments, if any, as operands YES
  • applications that call any utility with a first operand starting with "-" should usually specify "--" to mark the end of the options. YES
    Standard utilities that do not support this guideline indicate that fact in the OPTIONS section of the utility description.

in short:
Supported option syntax includes short and long form options:

     -a
    -bval
    -b val
   --noarg
   --witharg=val
   --witharg val

Repeating option

There is no standard for the repeating options in the POSIX/getopt and it's left for the implementer
Currently It's implemented like :

	 -c foo bar 
	 -c foo, bar
	 -c foo;bar  //based on the separator char you define

But the syntax

          -c foo -c bar      //is not implemented and it is nice  to be implemented in the library.

For example, in the downloader utility of vs2017, I can do:

     --add component1 --add component2   //not implemented, it's nice to be implemented
      --add component1  component2    //implemented

The standard syntax of the command line:

               utility_name [-abcDxyz][-p arg][operand]   //operand are the free args

@ericnewton76

derp -c "1" --asdf -c "2"

AFAIK, C may be IEnumerable<string> {"1","2"}, like what is implemented by Microsoft commandLine as described above (accumulative, not overwrite).

@duaneking

if a parameter argument is only required once and is used twice, the second usage overwrites the first one and thus the first argument is simply parsed, validated, and then ignored.

It's accepted if the parameter is defined as string not IEnumerable<string>

In the two cases parser shouldn't fire error and define the parameter value based on the data type scalar or sequence.

@tydunkel
Copy link
Contributor

Is this still being considered? This seems like an optional parser setting that we could opt-in to without breaking the old behavior. I have a potential fix that I could put a PR up for, but I want to make sure this is something that would be accepted.

@moh-hassan
Copy link
Collaborator

Welcome @tydunkel to the project.
What is the logic behind the Repeated options: for the scalar and sequence options?

@tydunkel
Copy link
Contributor

For scalars the last one wins. For sequences it will merge all options into one sequence. If the parser is opted-in to this behavior, the command line can only use multi-instance options for a sequence.

e.g. -c foo bar -c baz would fail, but -c foo -c bar -c baz would succeed if c is a sequence type

@moh-hassan
Copy link
Collaborator

It looks fine. You are welcome to submit PR.

@josesimoes
Copy link

+1 for having this!

@tydunkel I've peek your branch and you seem to already have this there. Can you submit that PR? 😉 😁

@rmunn
Copy link
Contributor

rmunn commented Mar 13, 2020

@josesimoes @moh-hassan #594 is a PR for @tydunkel's work on this feature. I haven't done anything to it except fix the merge conflicts with the current state of the develop branch and verify that all unit tests run.

@duaneking
Copy link
Author

The support people need is :

derp.exe -c foo-c bar

In this case either both foo and bar would be in the option list (in the case of supporting multiple values in that case) or its simply setting -c to the value of foo then ignoring that and setting the value of -c to bar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants