Support required and optional arguments

[tmtk75]

I supported a new field, Args, to cli.Command to specify required and optional arguments.

For example, we can write like this.

Name: "get",
Args: "<name> [id]",  // name is required, id is optional
Action: func(c *cli.Context) {
        name, _ := c.ArgFor("name")
        id, b := c.ArgFor("id")  // b is false if 2nd arg is not given
},

And the usage text appears like this.

USAGE:
   command get <name> [id]

I'm not sure whether the field name Args is good or not. I'm OK to change any names I added if you guys hope it.

Best,

[teamon]

Any chance to get this merged soon? This is the only thing missing for me in this great lib.

[kohkimakimoto]

👍
I hope that this PR is merged.

[julienvey]

This is a great feature. I hope it can be merged soon

[luan]

+1

[jawher]

Not very helpful, but here's another +1 for this PR to get merged !

[aussielunix]

@tmtk75 This looks really handy but am I right thinking it only works on subcommand arguments ?

[tmtk75]

@aussielunix Yes, you're right. It doesn't work without subcommands.
Do you hope like this?
https://gist.github.com/tmtk75/e39c75a72f373689ac45

    app := cli.NewApp()
    app.Name = "foo"
    app.Args = "<id> [path] [name]"
    app.Action = func(c *cli.Context) {
        id, _ := c.ArgFor("id")
        path, a := c.ArgFor("path")
        name, b := c.ArgFor("name")
        fmt.Printf("%v, %v:%v, %v:%v\n", id, path, a, name, b)
    }
    app.Run(os.Args)

[aussielunix]

@tmtk75 Baically. yes.
I want to be able to set a mandatory argument and have it show up in the --help.
It would be a nice to have to have optional arguments that show up in the --help

I still don't quote know enough golang to hack on this myself otherwise I would.

[tmtk75]

@aussielunix I tried to implement like tmtk75@34d519c

I want to be able to set a mandatory argument and have it show up in the --help.
It would be a nice to have to have optional arguments that show up in the --help

Then, help message of https://gist.github.com/tmtk75/e39c75a72f373689ac45 shows next.

tsakuma@~$ go run cmd.go -h
NAME:
foo - A new cli application

USAGE:
foo [global options] command [command options] <id> [path] [name]
...

<..> is required (id), and [...] is optional (path & name).
I think this USAGE text is enough to show mandatory and optional.

---

But there is a problem that help subcommand is automatically enabled.
go run cmd.go help shows help. The 1st example in Getting Started also works like that, too.
I think this is not expected for you.

I've only focused on subcommands, and it's a little bit tough to fix this by me... sorry.

[jszwedko]

@tmtk75 I like this idea, but I'm not a huge fan of "stringly typed" fields. What do you think about creating a cli.Arg type (similar to cli.Flag) and having app.Args be a list of those?

[gravis]

A must have. We are putting everything as flags now, otherwise it's undocumented :(
Thanks

[lucas-clemente]

What's the status of this PR? The issue has come up quite a few times, and it would be really awesome to see this merged :)

[jszwedko]

@lucas-clemente I'd like to see the removal of the stringly-typed fields before this goes in.

[lucas-clemente]

@tmtk75 Do you wanna update your PR, or should I make a fork and try to fix it?

[tmtk75]

@lucas-clemente I wish I could do by myself. It would be great if you could do that.

[stevvooe]

I like the concept of adding this feature but the sugary string syntax puts me off. They should be specified similarly to flags. Check out argparse for an example of this done well.

[lucas-clemente]

Thanks for the link @stevvooe, I agree. I will have a look and see what I can do next week. Might take a bit longer since I'm travelling :)

[pdf]

I'd be very keen to see this feature landed, but I don't think this is the right implementation. I think it should probably be based on the flags implementation.

This API might look something like:

app := cli.NewApp()
app.Name = `myapp`
app.Commands = []cli.Command{
  {
    Name: `get`,
    Args: []cli.Arg{
      cli.StringArg{
        Name:     `name`,
        Required: true,
        Usage:    `container to get from`,
      },
      cli.IntSliceArg{
        Name:     `id`,
        Usage:    `get specific id, may be specified multiple times`,
      },
    },
    Action: func(c *cli.Context) {
      fmt.Println(
        c.String(`name`),
      )
      fmt.Println(strings.Join(
        c.IntSlice(`id`),
        `,`,
      ))
    },
  },
}
...

And subcommand help might look something like:

NAME:
   get

USAGE:
   command get [command options] <name> [id [id...]]

ARGUMENTS:
   name      required        container to get from
   id        optional        get specific id, may be specified multiple times

In this implementation, flags and args would share the same keyspace, so it would be an error to declare an arg with the same name as a flag. Alternatively, a new set of retrieval commands could be implemented. It would be an error to define a required arg after an optional arg. Also, to support arbitrarily repeated arguments, it would be an error to declare more than one slice arg, and it would be an error to declare a slice arg anywhere other than the last position.

I'm not sure how to represent this output for the top-level app though, because the interplay of args and subcommands is already confusing. Probably the simplest way to handle it would be to output two lines of usage for the top-level app, one for subcommands, and one for natural args, ie:

NAME:
   myapp

USAGE:
   myapp [global options] command [command options] [arguments...]
   myapp [global options] <required-arg> [optional-arg]

COMMANDS:
   get

It would also be nice if cli provided an option to disable subcommands altogether for apps that don't require them, but that's a separate issue.

[stevvooe]

@pdf 👍 on your design. I would even add a capture spec field with values like '*', '?', '+' or a number to influence how many arguments are captured and the resulting documentation.

[pdf]

@stevvooe do you have a use-case for the capture spec? I feel like that could add a fair bit of complexity and possible ambiguity, without offering anything that can't already be achieved using the proposed syntax above - since greedy matching can only be performed on the last arg, all other args can simply be named individually.

I should also say that I'm not particularly happy with the error conditions in my proposal because they'll probably have to produce panics at runtime, but no way around that springs to mind.

[stevvooe]

@pdf Check out argparse for examples of use cases. In python, I used this all the time.

[tmtk75]

@pdf @stevvooe I'd like to close this pull request for now because this PR seems not welcome for you guys 😓 Could you create a new PR or issue and continue to discuss there? I think it's better for coming discussion and review.
I hope the new one will be merged soon 👍

[stevvooe]

@tmtk75 I'd leave that up to the maintainers. I'm just a guy on the internet with an opinion.

Your PR is probably a fine start. I'd just suggest moving away from the "sugary" arg specification. That style would be reserved for something like docopt.

[tmtk75]

@stevvooe I'm OK to leave this one actually.

Your PR is probably a fine start.

I'm happy if so 👍

I think it's better for coming discussion and review.

My point is the above comment.
The direction to implement seems to be determined with the design provided by @pdf
and @jszwedko, who is collaborator, doesn't prefer my one. So I think it doesn't make sense to continue on this pull request.

btw, I don't know python culture well, but docopt is interesting to me. Thanks for giving me it 😃

[pdf]

I'd like to get some input from maintainers on the design before any work goes ahead, so let's get some input before any decisions are made.

[jawher]

I have been following this PR for a while now, and I'd like to raise the following issue: parsing arguments is hard. Really hard.

Here are couple of tough cases:

Optional arguments before required ones

Something like git branch -m [OLD_NAME] NEW_NAME

Which might look harmless, but needs a backtracking mechanism to handle correctly in the general case

Non last repeatable arguments

Something like cp SRC... DST

Same as above, needs a backtracking parser.

Arguments dependent on options

Something like git branch (-m [OLD] NEW) | -a | (-d BRANCH)

More on the problem at hand in this blog post

Conclusion

So, while I agree with @pdf's proposal which adheres more to cli's way of doing things, it is simply not expressive enough to express the examples listed above.

Out of frustration with cli's lack of arguments support, I went ahead and wrote another go library to support a sugary syntax for expressing arbitrarily complex command invocation syntaxes (mow.cli). I had to use a full-blown finite state machine construction and traversal with backtracking to correctly handle the previous tricky cases (more info in this blog post)

So, my 2 cents: beware the innocent-looking required attribute on arguments and the ArgSlice for they might lead to this beauty:

[ARG1] ARG2 [ARG3] ARG4... ARG5

[pdf]

@jawher agreed, which is why I proposed a simple model that ignores those cases (well, panics at runtime if you break the rules). I'm not thrilled with it as a solution, but the alternative is pretty hairy as you rightly point out. That said, it certainly wouldn't be out of the question to implement the more complex patterns, but that's not something I have an interest in tackling any time soon.

[jszwedko]

Yeah, parsing arguments is definitely a difficult problem. I'd be OK with a naive solution that enforced constraints on how the arguments are laid out with respect to required vs optional and no accumulative arguments which could then be iterated upon.

[jszwedko]

It will definitely introduce a lot of complexity eventually though -- I think I might actually like to see it implemented as a separate package (within cli). That way we segregate some of the complexity as well as provide a useful package that can be used without cli. Something that takes a specification of the arguments and a []string.

[jszwedko]

Thank you for enumerating those points @jawher -- that blog post was really interesting too.

[langston-barrett]

+1 - I'd really love to see some kind of basic support for this. Is there anything I can do right now to help?

[gucki]

https://github.com/jawher/mow.cli Works just great! Thank you so much!

[jszwedko]

@siddharthist basically I'd like to see a subpackage within cli that offers an interface for parsing arguments from a []string and some format specification (preferably with concrete types like StringArgument).

[lukasmartinelli]

I use this trick to work around it.

func importJSON(c *cli.Context) {
    cli.CommandHelpTemplate = strings.Replace(cli.CommandHelpTemplate, "[arguments...]", "<csv file>", -1)

filename := c.Args().First()
    if filename == "" {
        cli.ShowCommandHelp(c, "json")
        os.Exit(1)
    }
    //continue
}

[jdonboch]

What happened with this function? Is there a way to do this in the latest version?

[charneykaye]

+1

[willneumob]

Is there a good workaround? I'm surprised this isn't implemented yet :(

[jszwedko]

@willneumob the current workaround is to do the custom argument validation after flag parsing. I unfortunately haven't had the time to dig into an implementation of this feature -- as indicated in the conversation above, it would be rather complicated -- but I am more than happy to review a pull request introducing it.