argparse icon indicating copy to clipboard operation
argparse copied to clipboard

README.md does not properly describe subcommands

Open gizmomogwai opened this issue 2 years ago • 3 comments

e.g. for me --help does not print the subcommands if I do not Annotate their SumType with @SubCommands. Also the first subsection reads Subcommands without UDA which indices for me, that there is a way to do subcommands without and with uda ... but its kind of the same, just the subcommands are configured with udas ...

gizmomogwai avatar Sep 20 '22 21:09 gizmomogwai

argparse supports two use cases:

  • without UDAs meaning no PositionalArgument, NamedArgument, SubCommands
  • with UDAs meaning that only annotated members are considered

The switch is: if there is at least one member with PositionalArgument, NamedArgument or SubCommands then the second approach is used.

andrey-zherikov avatar Sep 20 '22 22:09 andrey-zherikov

@gizmomogwai any objections if I close this in favor of #78? Both are about mixing *Argument and Sub Commands UDAs

andrey-zherikov avatar Sep 20 '22 23:09 andrey-zherikov

No problem ... please close... I was not sure if this is one or two issues :)

gizmomogwai avatar Sep 21 '22 00:09 gizmomogwai

argparse supports two use cases:

  • without UDAs meaning no PositionalArgument, NamedArgument, SubCommands
  • with UDAs meaning that only annotated members are considered

The switch is: if there is at least one member with PositionalArgument, NamedArgument or SubCommands then the second approach is used.

I did a lot of refactoring recently and found that this approach makes the code more complex - it adds dependency between unrelated code and makes possible enhancements a bit harder (if there is a need to add another UDA, for example).

So I think that the correct behavior going forward would be to make PositionalArgument/NamedArgument and SubCommands UDAs independent. But since this introduces change in library behavior, it make sense to have it in new major version 2.0. In addition to this, I'd like to review subcommand parsing and make it more clear and flexible.

As for this PR, I'm going to improve readme to make subcommands usage more clear (I just realized that @SubCommands is not mentioned in readme).

andrey-zherikov avatar Mar 28 '23 02:03 andrey-zherikov

thanks a lot. if you are planning on a new major version, feel free to discard this issue, as i would like to the your new stuff!!!

gizmomogwai avatar Apr 05 '23 20:04 gizmomogwai

I have an idea about how subcommand description can be improved and want to bring it up for a discussion.

I see no big value in having SubCommands UDA besides the fact that it "marks" a member that stores a chosen subcommand. On the other side, there is a legit use case when no subcommand is chosen: for example, it can be optional. And this use case is not possible with SumType as it chooses the first type as .init value.

So my idea is to introduce a new SubCommand type that will behave like a SumType with an addition: it will have a "no subcommand is chosen" state. Basically, SubCommand!(Cmd1, Cmd2) will mean something like SumType!(None, Cmd1, Cmd2) under the hood. In addition to that, introducing this new type makes existing SubCommands UDA obsolete which will make structs introspection a bit easier.

andrey-zherikov avatar Nov 22 '23 04:11 andrey-zherikov

And this use case is not possible with SumType as it chooses the first type as .init value.

Could one say, that the default subcommand needs to be the first type of the sumtype?

gizmomogwai avatar Nov 22 '23 07:11 gizmomogwai

And this use case is not possible with SumType as it chooses the first type as .init value.

Could one say, that the default subcommand needs to be the first type of the sumtype?

This would work for the case with default subcommand but not for the case without it.

Consider also the case when a command line does not specify a subcommand even when there is no default subcommand. This is also a legit use case, for example: tool --help or tool --version - in these cases no subcommand is specified and so data member should be None, not the first command (e.g. Cmd1).

andrey-zherikov avatar Nov 22 '23 14:11 andrey-zherikov

I see ... you are right ... I think this could work out ...

gizmomogwai avatar Nov 22 '23 14:11 gizmomogwai

@gizmomogwai I created new PR #143 to solve this issue (it replaces @SubCommands SumType!(...) with SubCommand!(...)). I will appreciate if you have time to take a look and provide your feedback.

andrey-zherikov avatar Dec 18 '23 04:12 andrey-zherikov

Hi @andrey-zherikov , thanks for the update!! I tried to update one of my "bigger" commandline tools to the newest version (g9c7bf6f). Observations:

  • I use fully qualified imports and it looks a little ugly now (although that's subjective)
import argparse.api.argument : NamedArgument, Description;
import argparse.api.argumentgroup : ArgumentGroup;
import argparse.api.command : Command, Epilog, Description;
import argparse.api.subcommand : SubCommand, Default;
import argparse.config : Config;
import argparse.api.ansi : ansiStylingArgument;

especially the two imports to Description look not too nice.

  • Default! seems not to work for me
    SubCommand!(Default!Review, Upload, Execute, Log) subcommand;

perhaps I used it wrong.

  • I am missing that the commandline shows the defaults for the parameters if there are any (probably hard without another uda, as it might be difficult to distinguish between init and real default values).
  • It's not clear for me with what to compare the color value that exposes the ansi coloring to the user (converting to bool worked best for me though). I could not get the comparison with Config.StylingMode or AnsiStylingMode working.
  • Link in the table of contents of readme.md to subcommands is broken.

Besides that everything seemed to work.

gizmomogwai avatar Dec 18 '23 10:12 gizmomogwai

  • I use fully qualified imports and it looks a little ugly now (although that's subjective)
import argparse.api.argument : NamedArgument, Description;
import argparse.api.argumentgroup : ArgumentGroup;
import argparse.api.command : Command, Epilog, Description;
import argparse.api.subcommand : SubCommand, Default;
import argparse.config : Config;
import argparse.api.ansi : ansiStylingArgument;

especially the two imports to Description look not too nice.

Did you try using import argparse?

import argparse : ArgumentGroup,
                  Command,
                  Config,
                  Default,
                  Description,
                  Epilog,
                  NamedArgument,
                  SubCommand,
                  ansiStylingArgument;
  • Default! seems not to work for me
    SubCommand!(Default!Review, Upload, Execute, Log) subcommand;

perhaps I used it wrong.

Could you please provide a test case? I checked with example and it seems working.

  • I am missing that the commandline shows the defaults for the parameters if there are any (probably hard without another uda, as it might be difficult to distinguish between init and real default values).

There shouldn't be any changes in this area. Could you please provide an example?

  • It's not clear for me with what to compare the color value that exposes the ansi coloring to the user (converting to bool worked best for me though). I could not get the comparison with Config.StylingMode or AnsiStylingMode working.

Do you mean a case to determine whether styling is enabled/disable at runtime? This was changed since last 1.* release. Below is a part of release notes draft for 2.0 - please let me know whether your use case is different.

  • Underlying type of ansiStylingArgument argument is changed. It can now be directly cast to boolean instead comparing against Config.StylingMode.

    So if you use it:

      static auto color = ansiStylingArgument;
    

    then you should replace

      if(args.color == Config.StylingMode.on)
    

    with

      if(args.color)
    

andrey-zherikov avatar Dec 18 '23 23:12 andrey-zherikov

  • I use fully qualified imports and it looks a little ugly now (although that's subjective)
import argparse.api.argument : NamedArgument, Description;
import argparse.api.argumentgroup : ArgumentGroup;
import argparse.api.command : Command, Epilog, Description;
import argparse.api.subcommand : SubCommand, Default;
import argparse.config : Config;
import argparse.api.ansi : ansiStylingArgument;

especially the two imports to Description look not too nice.

Did you try using import argparse?

import argparse : ArgumentGroup,
                  Command,
                  Config,
                  Default,
                  Description,
                  Epilog,
                  NamedArgument,
                  SubCommand,
                  ansiStylingArgument;

that works for me and i will use that now. usually i like to know where things are really implemented (just not for std.algorithm :-)) thats why i used the specialized imports.

  • Default! seems not to work for me
    SubCommand!(Default!Review, Upload, Execute, Log) subcommand;

perhaps I used it wrong.

Could you please provide a test case? I checked with example and it seems working. I will try to come up with a reduced example:

import argparse;
import std.stdio: writeln;


struct sum {}
struct min {}
struct max {}

struct Program
{
    int[] numbers;  // --numbers argument

    // name of the command is the same as a name of the type
    SubCommand!(Default!sum, min, max) cmd;
}

// This mixin defines standard main function that parses command line and calls the provided function:
mixin CLI!Program.main!((prog)
{
    static assert(is(typeof(prog) == Program));

    int result = prog.cmd.match!(
        (.max)
        {
            import std.algorithm: maxElement;
            return prog.numbers.maxElement(0);
        },
        (.min)
        {
            import std.algorithm: minElement;
            return prog.numbers.minElement(0);
        },
        (.sum)
        {
            import std.algorithm: sum;
            return prog.numbers.sum;
        }
    );

    writeln("result = ", result);

    return 0;
});

here i would expect, that sum is used, when no command is given. but for me this is not the case.

  • I am missing that the commandline shows the defaults for the parameters if there are any (probably hard without another uda, as it might be difficult to distinguish between init and real default values).

There shouldn't be any changes in this area. Could you please provide an example? There is no change, its just a feature that i would like to have. E.g. in the above numbers example if I replace the numbers line with

int[] numbers = [1, 2, 3];  // --numbers argument

then I would like to see to what numbers defaults, if i do not put it on the commandline.

  • It's not clear for me with what to compare the color value that exposes the ansi coloring to the user (converting to bool worked best for me though). I could not get the comparison with Config.StylingMode or AnsiStylingMode working.

Do you mean a case to determine whether styling is enabled/disable at runtime? This was changed since last 1.* release. Below is a part of release notes draft for 2.0 - please let me know whether your use case is different.

  • Underlying type of ansiStylingArgument argument is changed. It can now be directly cast to boolean instead comparing against Config.StylingMode. So if you use it:
      static auto color = ansiStylingArgument;
    
    then you should replace
      if(args.color == Config.StylingMode.on)
    
    with
      if(args.color)
    

I did that, and it worked for me. I also like the api, but i was just a little surprised ...

thanks for the reply

kind regards, christian

gizmomogwai avatar Dec 19 '23 08:12 gizmomogwai

here i would expect, that sum is used, when no command is given. but for me this is not the case.

I added writeln("prog=",prog); right after static assert in main and here what I see:

$ ./app --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

$ ./app max --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(max()))
result = 3

$ ./app min --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(min()))
result = 0

$ ./app sum --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

Do you mean the case when there is no parameters in command line?

$ ./app
prog=Program([], SubCommand!(Default!(sum), min, max)(None()))
result = 0

There is no change, its just a feature that i would like to have. E.g. in the above numbers example if I replace the numbers line with

int[] numbers = [1, 2, 3];  // --numbers argument

then I would like to see to what numbers defaults, if i do not put it on the commandline.

Could you please create a separate issue with examples of what you want to see?

I did that, and it worked for me. I also like the api, but i was just a little surprised ...

Sorry for inconvenience. master has now breaking changes comparing with 1.* releases and I have draft release notes that I can't make public without cutting a release which I'm not ready to do yet.

andrey-zherikov avatar Dec 19 '23 15:12 andrey-zherikov

here i would expect, that sum is used, when no command is given. but for me this is not the case.

I added writeln("prog=",prog); right after static assert in main and here what I see:

$ ./app --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

$ ./app max --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(max()))
result = 3

$ ./app min --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(min()))
result = 0

$ ./app sum --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

Do you mean the case when there is no parameters in command line?

$ ./app
prog=Program([], SubCommand!(Default!(sum), min, max)(None()))
result = 0

yes exactly ...

There is no change, its just a feature that i would like to have. E.g. in the above numbers example if I replace the numbers line with

int[] numbers = [1, 2, 3];  // --numbers argument

then I would like to see to what numbers defaults, if i do not put it on the commandline.

Could you please create a separate issue with examples of what you want to see? yes ... will try to come up with an issue for that ... it's just a wish

I did that, and it worked for me. I also like the api, but i was just a little surprised ...

Sorry for inconvenience. master has now breaking changes comparing with 1.* releases and I have draft release notes that I can't make public without cutting a release which I'm not ready to do yet. no problem, it is really nice now! I did not want to complain (I know what it means, to take something from a branch-in-progress) or from a major update.

gizmomogwai avatar Dec 19 '23 21:12 gizmomogwai

Do you mean the case when there is no parameters in command line?

import argparse;

struct status {
    bool all;
}

struct start { }
struct stop { }

struct Args {
    SubCommand!(Default!status, start, stop) sub;
}

mixin CLI!Args.main!((args) {
    import std.stdio;

    args.sub.match!(
        (status st) {
            write("Querying status (all = ", st.all, ").\n");
        },
        (.start) {
            write("Starting.\n");
        },
        (.stop) {
            write("Stopping.\n");
        },
    );
});
$ ./prog status --all
Querying status (all = true).
$ ./prog status
Querying status (all = false).
$ ./prog --all
Querying status (all = true).
$ ./prog
$

The last invocation produces no output at the moment. This seems counter-intuitive to me.

In my opinion, it should set the subcommand field to default in this case, too. Alternatively, match can invoke the default handler when the subcommand is none, but I’m uncertain whether it is a good idea.

SirNickolas avatar Dec 21 '23 11:12 SirNickolas

I've fixed this although not in a way as I thought initially how SubCommand would work (I thought that we shouldn't initialize a member to be "default" command).

andrey-zherikov avatar Dec 21 '23 16:12 andrey-zherikov

Consider also the case when a command line does not specify a subcommand even when there is no default subcommand. This is also a legit use case, for example: tool --help or tool --version - in these cases no subcommand is specified and so data member should be None, not the first command (e.g. Cmd1).

--help is handled by the framework via internal magic, thus it shouldn’t bother users. --version is more interesting though. There are CLI libraries that make it magic as well, but we do not (at the moment, at least). I agree it is better to provide a flexible solution than add more hard-coded arguments so let’s try. The question is, how should we achieve that.

To be honest, the idea of adding None to every subcommand doesn’t really appeal to me. It feels like adding null to the domain of pointers. Whether a user needs it or not, they are forced to deal with a possibility that a command may be absent – and if it must not be absent, the user should emit an error manually. I believe the only thing that should be responsible for validating the command-line syntax and producing errors is the CLI framework.

Let’s consider an example that I find challenging. Suppose we have a program that can be invoked in several ways:

Usage:
  ./prog [--verbose] show WHAT
  ./prog [--verbose] [move] WHAT WHERE
  ./prog [--verbose] --version

(By the way, this is a correct machine-readable specification in the docopt format.) move is the default command. In addition, ./prog --version is allowed but ./prog (no arguments) and ./prog WHAT (a single argument) are not. How do we declare such CLI?..

I see a simple solution that is somewhat restricted but works for --version-like arguments.

struct show {
    @PositionalArgument(0) string what;
}

struct move {
    @PositionalArgument(0) string what;
    @PositionalArgument(1) string where;
}

struct Args {
    SubCommand!(show, Default!move) sub;
    @NamedArgument bool verbose;
    @(NamedArgument("version").LiftRequirements(true)) bool version_;
}

When an argument with .LiftRequirements is given on the command line, argparse will not check if required arguments (named/positional) are present. Therefore, a user will inspect version_ first, print the version if it is true, or switch on sub otherwise.

It solves the problem, though I admit it’s not especially elegant. A more complete solution would be extending RestrictionGroups to cover this use case. I think it is possible somehow but have no clear understanding at the moment how exactly.

SirNickolas avatar Dec 30 '23 21:12 SirNickolas

here i would expect, that sum is used, when no command is given. but for me this is not the case.

I added writeln("prog=",prog); right after static assert in main and here what I see:

$ ./app --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

$ ./app max --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(max()))
result = 3

$ ./app min --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(min()))
result = 0

$ ./app sum --numbers 1 2 3
prog=Program([1, 2, 3], SubCommand!(Default!(sum), min, max)(sum()))
result = 6

Do you mean the case when there is no parameters in command line?

$ ./app
prog=Program([], SubCommand!(Default!(sum), min, max)(None()))
result = 0

yes exactly ...

I checked this example with the HEAD of PR. Everything works as extected:

$ dmd -i -I../source/ -run app.d
prog=Program([], SubCommand!(Default!(sum), min, max)(sum()))
result = 0

andrey-zherikov avatar Jan 13 '24 03:01 andrey-zherikov

Do you mean the case when there is no parameters in command line?

import argparse;

struct status {
    bool all;
}

struct start { }
struct stop { }

struct Args {
    SubCommand!(Default!status, start, stop) sub;
}

mixin CLI!Args.main!((args) {
    import std.stdio;

    args.sub.match!(
        (status st) {
            write("Querying status (all = ", st.all, ").\n");
        },
        (.start) {
            write("Starting.\n");
        },
        (.stop) {
            write("Stopping.\n");
        },
    );
});
$ ./prog status --all
Querying status (all = true).
$ ./prog status
Querying status (all = false).
$ ./prog --all
Querying status (all = true).
$ ./prog
$

The last invocation produces no output at the moment. This seems counter-intuitive to me.

In my opinion, it should set the subcommand field to default in this case, too. Alternatively, match can invoke the default handler when the subcommand is none, but I’m uncertain whether it is a good idea.

I checked this also - works as expected:

$ ./prog status --all
Querying status (all = true).
$ ./prog status
Querying status (all = false).
$ ./prog -all
Querying status (all = true).
$ ./prog
Querying status (all = false).

andrey-zherikov avatar Jan 13 '24 03:01 andrey-zherikov

Let’s consider an example that I find challenging. Suppose we have a program that can be invoked in several ways:

Usage:
  ./prog [--verbose] show WHAT
  ./prog [--verbose] [move] WHAT WHERE
  ./prog [--verbose] --version

The idea I have in mind for --version argument:

@NamedArgument("version") void version_() { writeln("Version 1.2.3"); return Result.ForceExit(0); }

This seems pretty simple and will be useful for reducing internal magic for --help.

andrey-zherikov avatar Jan 13 '24 04:01 andrey-zherikov