Option to parse option without any switch (position based) about commandline HOT 41 CLOSED

Hi Konrad,
thank you for writing and your interest in the library.

The library was built around named options, but as it grown (both on code- and user- base) unnamed options were also managed.

At the moment you can catch an arbitrary (or well defined) number of unnamed values using ValueListAttribute. It allows you to set a constraint over the maximum number of unnamed values mapped in a IList<string> property.

Recent version also introduced ValueOption. It works for scalar values. Example:

class WebServerOptions {
  [ValueOption]
  int Port { get; set; }
  [ValueOption]
  public string DocumentRoot { get; set; }
}

Values are mapped respecting order of declaration (in code) and are always optional, so:

# this will work
$ websrv 8080 ~/var/www/localtests
# but this will let ParseArguments returns false (string -> int; bad thing!)
$ websrv ~/var/www/localtests 8080
# but the following works!
$ websrv

I think that you want something like this (THAT ACTUALLY DO NOT EXISTS!!!)

class WebServerOptions {
  [ValueOption Required=true]
  public int Port { get; set; }
  [ValueOption Required=true]
  public string DocumentRoot { get; set; }
}

But how will work with if you mix required and not required ValueOptions? I don't know... I've to think on it. Maybe we can give it an index. And such index works independently for unnamed options?
Something like:

class WebServerOptions {
  [ValueOption Index=0] //Setting an index means also that that the option is required
  public int Port { get; set; }
  [ValueOption Index=1] //I can also add "Index" as a required constructor value...
  public string DocumentRoot { get; set; }
}

I've to think on it.

Anyway also if more power on managing unnamed values will be cool, I think that the library can actually handle most of input scenario. Am I wrong?

Give a look also to this thread (#33) with two of our main contributors @mizipzor and @nemec.

Regards, Giacomo

from commandline.

@konrad-kruczynski, please continue discussion here. Issue #33 is closed (it was only mentioned for reference).

What do you think (others invited too...) to choose the solution of the index property:

class WebServerOptions {
  [Option("conn", Required=True)]
  public int MaxConnections { get; set; }
  [ValueOption RelativeIndex=0]
  public int Port { get; set; }
  [ValueOption RelativeIndex=1]
  public string DocumentRoot { get; set; }
}

I've changed the hypothetical name of Index to RelativeIndex for demonstrate the following:

# every line of following shell input is valid
$ websrv --conn 1000 8080 ~/var/www/mytests/
$ websrv 8080 ~/var/www/mytests/ --conn 1000
$ websrv 8080 --conn 1000 ~/var/www/mytests/

Why this input is still valid? Because ValueOptions are indexed in a relative way between them, independently from other [Option*]s. Hece the name RelativeIndex -> relative only to ValueOptions.

Opinions?

from commandline.

Exacly what I need. As I presume, DocumentRoot could also be optional (unless another required Option was after it).

from commandline.

@konrad-kruczynski, yes if defined without index:

class WebServerOptions {
  [Option("conn", Required=True)]
  public int MaxConnections { get; set; }
  [ValueOption RelativeIndex=0]
  public int Port { get; set; }
  [ValueOption]
  public string DocumentRoot { get; set; }
}

So we can put on a big red uppercase print: "APPROVED!" ?

from commandline.

Hmm, one note tough: I think that the index should always be required, no matter if the given option is optional. This is due to the ordering problem discussed earlier, i.e. that the properties queried using reflection are returned in unspecified order. With only one optional parameter this does not pose a problem, but if there were two of them, we couldn't tell which belongs to which parameter value given in the command line. So I'd probably like something like:

class WebServerOptions {
  [ValueOption(0)]
  public int Port { get; set; }
  [ValueOption(1, Optional = True)]
  public string DocumentRoot { get; set; }
  [ValueOption(2, Optional = True)]
  public int AnotherOption { get; set; }
}

And some valid program calls:

program 1234
program 1234 ./docs
program 1234 ./docs 888

but not:

program 1234 888

from commandline.

Oh, naturally the indices may be numbered from 0 :)

from commandline.

I'm gonna have to disagree here. If you ask me, all positioned options should be required.
Consider your last example:

program 1234 888

To me, its valid. If your document root is called 888.

The point being, how would the program know? If any positioned option is optional, it would be the last one and only the last one.

from commandline.

Ok, indeed my last example was wrong. Nonetheless I think you can have more than one optional parameter, but all of them have to be located at the end and if you want to issue kth parameter, you have to issue all k-1 earlier. What do you think? (Similar to optional parameters in C++ functions).

from commandline.

Even if located at the end, how would the parser know what option I left out?

class Options {
  [ValueOption(0, Optional = True)]
  public string A { get; set; }
  [ValueOption(1, Optional = True)]
  public string B { get; set; }
  [ValueOption(2, Optional = True)]
  public string C { get; set; }
}

If invoked like program.exe foo bar, did I leave option B or C out? Or did I leave out A?

from commandline.

I mean that you won't be allowed to leave B without leaving C or leaving A without B and C. Just like in C++ if you have:

void fun(int a=0, int b=1, int c=2) { body; }

you cannot omit a if you want to specify b or c and so on. Hope it's clearer now :)

from commandline.

Right, so in my last example the parser would assume that I left out C because neither AC nor BC is a valid combination given the indexes.
Though it might be hard to communicate to the user. I'm currently trying to picture what the help text auto build will do in this case.

But I now see how this could be accomplished on a technical level in the code. And there are use-cases for it. You've convinced me. I approve.

from commandline.

@konrad-kruczynski, @mizipzor,
the first thing to take into account (for me) is one: made the API clearly understandable, easy to use, free from ambiguities.

@konrad-kruczynski, I'm sincerely a bit confused...

Why we don't try to summary simply how to evolve ValueOption and its behavior without in this context?

Should be required by default? Should by optional? Index is relative to other ValueOptions or not?

I want to be able to look at an Options class and say OK, this program accepts X, Y and Z is optional.

If too reasoning is required than (for me) we're creating something too difficult to use.

from commandline.

I would like to see an Index property added to ValueOption regardless. The main reason is that I work with a styleguide that require me to declare properties in alphabetical order. I'm currently forced to break this rule to make these indexed options work.

After reading this thread I would also like to see a IsRequired property added to ValueOption with the behaivor discussed. Edit: Or the inverted IsOptional, of course.

I would also like to stress that these two are separate features.

from commandline.

I think the ultimate design is rather simple: with valueoption you have to provide index (just because of reflection shortcomings and I would make index required, i.e. parameter of the attribute ctor) and IsRequired/IsOptional as @mizipzor mentioned. So, to answer your question:

Should be required by default?

IMO yes, so I favor for the IsOptional. IMO this is what user usually expects: named options are optional (so they are named), but required just have to be provided.

Index is relative to other ValueOptions or not?

I'm not sure whether I understood the question, but generally index is only to provide some deterministic order, so you can query the properties and sort by the index. Indices do not have to be consecutive or start from a given value.

IMO it should be readable, also look again at this code sample in #50 (comment)

from commandline.

To return to the original comment regarding grep which has the following usage:

grep [OPTIONS] PATTERN [FILE...]

As far as I'm concerned this currently works with one exception. There is no way to make Pattern or File required. I tried this:

class GrepOptions
{
    [Option('a', "text")]
    public bool Text { get; set; }


    [Option('c', "count")]
    public bool Count { get; set; }

    // And all the other grep option
    // ...

    // first and only indexed option
    [ValueOption]
    public string Pattern { get; set; }

    // "the rest", assuming list of file names
    [ValueList(typeof(List<string>))]
    public List<string> Files { get; set; }
}

class Program
{
    static void Main(string[] args)
    {
        // works as it should
        Test("-c foo* file.txt file2.txt".Split());

        // sadly, no way to make Pattern and Files required
        Test("--text".Split());
    }

    static void Test(string[] args)
    {
        GrepOptions options = new GrepOptions();
        Parser.Default.ParseArguments(args, options);
        Console.WriteLine(options.Text);
        Console.WriteLine(options.Count);
        Console.WriteLine(options.Pattern);
        Console.WriteLine(string.Join(" ", options.Files));
    }
}

This is kinda how the original grep behaves. If you:

1. Omit the File it starts to read from stdin.
2. Omit the Pattern it considers the File as the Pattern (you can also say that you're not allowed to omit Pattern).
3. Omit both Pattern and File, it prints usage (but the above code does not).

It should be up to the user of this library to implement 1. Regarding 2 its already supported. It seems to me what we're discussing here is how to support 3 in the library.

from commandline.

Very interesting... But this is done with latest release, right? No with your last pull request (that I'm going to check out now).

from commandline.

This is with your latest master. My pull request only allows you to reorder ValueOptions and can't do anything here since its only one.

from commandline.

Yup, and 3 does not work because currently one cannot make ValueOption required.

Apart from that I would still insist on making the index required (ctor) parameter to protect users from the reflection surprises... What do you think?

from commandline.

Have a look at #52, @nemec have also (strongly) suggested making index a ctor parameter.

from commandline.

@konrad-kruczynski what is your latest thought on this enhancement? @mizipzor provided a PR with mandatory Index (other ctor will be removed on next Stable, or event next RC0).

@mizipzor, please let me know also what you think.

We should add a Required property to enforce???

from commandline.

I'm fine with that :)

from commandline.

@konrad-kruczynski, ok I'm finishing passing few files to StyleCop and than I think I can start with.

from commandline.

@konrad-kruczynski
@mizipzor

Scheduled in next beta. Not too time, the actual code base will be promoted to stable as soon.

Remarks: a new model we be adopted for development. After master become stable, new branch called develop will be created.

Just for record.

from commandline.

Work is not stopped, please read https://github.com/gsscoder/commandline/wiki/Official-Roadmap

from commandline.

Since this can be accomplished with [Value(Index)] attribute of 2.0 pre-rel, are you agree to close the issue?
cc/ @konrad-kruczynski @mizipzor

from commandline.

Consider following scenario in which given index based option is only valid if given switch was not used. For example this is valid:

program index_arg

and this is valid:

program --some_switch index_arg

but this is not valid:

program --other_switch index_arg

In other words index_arg can only show up if --other_switch is not used.

Can I achieve that now?

from commandline.

@konrad-kruczynski Mmm... At the moment there's still no idiomatic way of constraining options in indexed positions.
(Probably something can be done or via adding manual checks or via mixing boolean values and unbound index values; but I discourage such practices since will complicate your code and all the parsing should be accomplished by the library).

As said as now only [Value] can be indexed.

If I've correctly understood your request... this could be considered a feature request.

from commandline.

My point is that indeed only [Value] can be indexed, but I'd like have some of these available only if a given switch is not used. What do you think?

from commandline.

I tried to read through this but the more I look at it the more confused I get. Let me doublecheck, the index_arg can change index depending on other switches, if neither --some_switch nor --other_switch is specified it has index 0, if only --some_switch is specified it has index 1, if --other_switch is specified it's not valid at all?

If all the above is correct, it sounds really complicated to me. Going back to the original example of Grep, is this really how it works?

from commandline.

I believe it has more to do with mutually exclusive sets: index_arg should be allowed only when other_switch is not present.

from commandline.

Are you talking in respect of a particular index? (Sorry maybe working only on commercial products so long made me a bit stupid...) :(

--some_switch, --other_switch is a boolean option: [Option(...)] -> set to boolean target property
index_arg is value with index: [Value(..)] -> set with index = 0
RULES:

• --some_switch can appear before the value with index = 0
• --other_switch can't, only after

This is can't be done now. But remember that index of values are not the same of command line tokes. Each token is mapped on specific value after all named options are parsed.

To my opinion what you require could complicate the parser and I'd like to hear more opinions before implementing something so specific... (Hope you understand).

The other question is: how can we implement custom rules like this in a general manner? With a lambda function?

from commandline.

Hi @mizipzor ! How much time, Alexander...

@mateusz-holenko Hi, happy you joined discussion!

I think that @mizipzor expressed something similar my last post, isn't it?

from commandline.

Yeah, the point is that software can, for example, execute script (given as an indexed argument) or execute a command (with e option). But one cannot use both options at the same time. So this is legal:

software.exe script_name

and this is legal:

software.exe -e command

but this is not:

software.exe -e command script_name

from commandline.

@mizipzor I think he want something the plug-in part added to latest stable by @gimmemoore ... The plug-in friendly architecture is explained here; but it's stable only for now...

from commandline.

Hm, this is starting to sound similar to subparsers or the concept of verbs. I think it was discussed somewhere... or was it even a pull request? I can't find it now, though.

from commandline.

@mizipzor you're talking of nesting multiple verb commands? that post?

from commandline.

And yup, regarding method to let the user communicate complex scenarios, the simplest one would probably be to accept some validating function. Here one would only need to extend existing mutual exclusion options up to indexed parameters.

from commandline.

@konrad-kruczynski kernel of 2.0 try to be as more functional as C# allows (at the moment of writing); so I vote up 👍 for a validating function in the concrete sense of the word.

@mizipzor ?

from commandline.

If you want my opinion in any official capacity I'll have to abstain from voting as I don't feel I understand the problem well enough anymore. If we can delegate validation to the implementer, sure. But I'm still struggling to see why you would want a program to behave this way, grep was the example initially but it doesn't seem to be anymore, can we try to find a command line tool that works like this?

from commandline.

@mizipzor Agree it's a too particular question...

But moving the discussion forward I think it could be useful allow some custom validation step using lambda functions... My 👍 was toward solving the general problem with this kind of architecture that marries with 2.0 kernel.

from commandline.

This is possible from 2.0.x+. So I close issue.

from commandline.