urfave / cli

A simple, fast, and fun package for building command line apps in Go
https://cli.urfave.org
MIT License
22.18k stars 1.7k forks source link

Allow flags to come after arguments #1950

Open fiatjaf opened 2 months ago

fiatjaf commented 2 months ago

Checklist

What problem does this solve?

cli -s file.txt and cli file.txt -s should work the same way because having to strictly type the flags before the arguments is annoying. Most unix utils allow flags after arguments so people are used to that pattern and a thing that is obviously a flag shouldn't be interpreted as an argument just because it comes after another argument.

Solution description

cli -s file.txt and cli file.txt -s should work the same way. cli -s file.txt and cli --foo=bar file.txt -s -v --whatever=something should work the same way.

Describe alternatives you've considered

I've tried switching to other CLI frameworks, but they are all much worse than this except in this aspect.

I've tried frontporting a feature from v1 that reordered arguments manually before parsing them: https://github.com/urfave/cli/pull/1928 (currently using this with a custom fork and replace).

XANi commented 2 months ago

Additional context:

With multi-command binaries it's pretty common pattern to do the "config the command" first:

cli --server 1.2.3.4 --password-file ~/.cli/production.creds

and then when using it to just put subcommand after

cli --server 1.2.3.4 --password-file ~/.cli/production.creds ls --help

cli --server 1.2.3.4 --password-file ~/.cli/production.creds show

cli --server 1.2.3.4 --password-file ~/.cli/production.creds show --filter asd

or outright alias it

alias cli-prod cli --server 1.2.3.4 --password-file ~/.cli/production.creds

cli-prod ls --help

cli-prod show

cli-prod --filter asd

"POSIX compatbile" is not good. People are not POSIX compatible.

BlackHole1 commented 2 months ago

In v3(alpha), this is already supported. You can verify it using: go get github.com/urfave/cli/v3.

Related PR: #1568 #1835

fiatjaf commented 2 months ago

It's not.

decentral1se commented 2 months ago

@BlackHole1 https://github.com/urfave/cli/issues/1717#issuecomment-1624383064 https://github.com/urfave/cli/issues/976#issuecomment-2056208381

dearchap commented 2 months ago

Additional context:

With multi-command binaries it's pretty common pattern to do the "config the command" first:

cli --server 1.2.3.4 --password-file ~/.cli/production.creds

and then when using it to just put subcommand after

cli --server 1.2.3.4 --password-file ~/.cli/production.creds ls --help

cli --server 1.2.3.4 --password-file ~/.cli/production.creds show

cli --server 1.2.3.4 --password-file ~/.cli/production.creds show --filter asd

or outright alias it

alias cli-prod cli --server 1.2.3.4 --password-file ~/.cli/production.creds

cli-prod ls --help

cli-prod show

cli-prod --filter asd

"POSIX compatbile" is not good. People are not POSIX compatible.

@XANi I dont see how your example relates to this issue.

dearchap commented 2 months ago

@fiatjaf Please make sure that there is a field that can be set in Command to enable the behavior. Also make sure you have test cases for

cmd subcmd1 subcmd2 -f -s -X

where subcmd1 defines these flags and redorder is configured ONLY on that subcommand. Unless you are strictly expected this usage only for single level subcmd. I anticipate that people will ask why it does not work for 2-3 level sub commands. This has to be thought out more thoroughly.

fiatjaf commented 2 months ago

This is my feature request, that PR is just one possible but humble attempt at implementing it. I'm sure people with more experience with this code can deal with this better, hence this feature request.

dearchap commented 2 months ago

@fiatjaf you may submit the PR and we can merge it but eventually over time it is upto the maintainers to maintain this code and move it forward. So I want to make sure we have a firm foundation since this is not a simple feature.

decentral1se commented 2 months ago

Thanks for weighing in @dearchap 🎉

I'm trying to think through an example which allows a ReorderFlags: true to be set on the root command which is then propagated to all sub-commands. If we take the following example:

# rough definition
cli [--global] sub1 [--foo] sub2 [--bar] sub3 [--baz] arg1

# user chaotically types while hacking
cli sub1 sub2 --foo sub3 --bar arg1 --baz --global

# transparently reordered
cli --global sub1 --foo sub2 --bar sub3 --baz arg1

Then the re-order logic is "just": parse flags and return them to the position where they are "registered", i.e. Flags: []cli.Flag{fooFlag}. There is no guessing what the user wants and we follow the logic of the *cli.Command.

We just need to figure out a precedence convention for flag names overlapping? Perhaps it is "local wins"?

Thoughts?

dearchap commented 2 months ago

@decentral1se Thank you for the example. The example supposes that these are bool flags correct ? If you had this

cli sub1 sub2 --foo 10 sub3 --bar arg1 --baz --global

for example we wouldnt reorder since foo is an int flag ? It starts getting tricky. So say we support only bool flags and we have shortoptionshandling enabled we would need to parse out

cli [--global] sub1 [-f] sub2 [-b] sub3 [-z] arg1
cli sub1 sub2 sub3 arg1 --global -fbz

to 

cli --global sub1 -f sub2 -b sub3 -z arg1

I think we need to start working on #1273 which would provide a foundation for this behavior. One of pre-exec functions would allow users to add flags dynamically and another of those pre-exec functions would do reordering.

decentral1se commented 2 months ago

@dearchap that's a good point about non-bool flags. Yeh, I guess it's just the same rule: follow what is registered on the flag (vs. the command this time) - does it accept an argument? then bring that with the sort? Will try manual test along with https://github.com/urfave/cli/pull/1928 great work @fiatjaf