reorder flags so they can come after arguments

[fiatjaf]

What type of PR is this?

• feature

What this PR does / why we need it:

This brings back (and improves) the reorderFlags function from v1 that allows flags to be specified after arguments and still work. It's a very much requested feature, see https://github.com/urfave/cli/issues/1733 and https://github.com/urfave/cli/issues/976.

Which issue(s) this PR fixes:

Fixes #976

Testing

I've done some manual testing.

[skelouse]

I did something similar in another repo if you need this: https://github.com/urfave/cli/issues/1583#issuecomment-1385733549

[linrl3]

Some Run tests are failed, please check @fiatjaf

[fiatjaf]

I don't understand what is failing. Looks like it's test coverage only? I can fix that, but can you tell me first if this change is wanted and would be merged if tests were ok?

[dearchap]

@fiatjaf No one has specifically asked for this feature, so it depends. Also we have persistent flag support, so flags can be defined so that they can be placed anywhere in the chain

[fiatjaf]

I've linked to two issues above with many people asking for it. I've seen you replied on these issues with that comment about persistent flags, but I don't think it's related.

This is about specifying flags after arguments, not after subcommands. Like cli -s file.txt and cli file.txt -s should work the same way.

[joshfrench]

This trips up my team all the time — I’d definitely appreciate the feature!

[dearchap]

@fiatjaf This starts getting complicated. Can you provide more examples of the behaviour ? if you have say

cli subcmd1 subcmd2 -f -s -t

that would be equivalent to

cli -f -s -t subcmd1 subcmd2

even if say '-s -t' are defined for subcmd1 and not root command.

[fiatjaf]

Subcommands should not be involved here. So cli subcmd -s -t -m file.txt otherfile.txt should be equivalent to cli subcmd file.txt otherfile.txt -s -t -m.

[dearchap]

I understand the appeal of this but you are interpreting what the user want to achieve. Its is entirely possible that the subcmd wants to handle all the args itself in the command action.

cli subcmd file.txt s.txt -f -s

So you need a new boolean field in Command, something like ReorderArgs, which is off by default and can be enabled if a particular user of this library wishes it. Also you have to ensure that the reorder can be applied only to a leaf command and not to root/branch commands which have subcommands.

[decentral1se]

No one has specifically asked for this feature

checks notes one of the most critically requested features and reason for several projects migrating back to v1 and/or dropping urfave/cli altogether in favour of cobra 😆

[abitrolly]

checks notes one of the most critically requested features and reason for several projects migrating back to v1 and/or dropping urfave/cli altogether in favour of cobra 😆

[reference needed]. I don't see "reorder" in the critical list of requested features.

https://github.com/urfave/cli/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc

Alzo the v1 migration dataset is very interesting to see.

[decentral1se]

@abitrolly you don't have to look far https://github.com/urfave/cli/issues/1113 Also see what @fiatjaf referenced? There are several folks (including myself) who had to remigrate to v1. Additionally, several folks there and in other issues (use search) migrated to cobra because of this. I don't know who curates this list but this ordering issue should be on it imho

Also, https://github.com/urfave/cli/issues/1331#issuecomment-1248422899 "its a long standing feature ask"

[fiatjaf]

I've opened a feature request here: https://github.com/urfave/cli/issues/1950

Let's go there an upvote it!

[XANi]

Can confirm, can't move to v2 in any of our project for ergonomic issues. "It's more POSIX compliant" is a very weak arguments, humans are not POSIX compliant.

We have a lot of pattern that can be described as

<cmd> --global --config <subcmd> --subcmd-config

because for CLI user it is easier to just copy front part (<cmd> --global --config ) or outright alias it for shorter cmdnline and then change subcommand and its options when using it.

Forcing flags to be at certain position just means unnecesary cursor movement

[decentral1se]

I've done manual some testing @fiatjaf and it doesn't seem to be working for flags on sub-commands?

./<cli> <command> --help # (1)
./<cli> <command> <sub-command> --help # (2)

Where both (1)/(2) show help output for <command> only.

I can't seem to trigger help output for <sub-command> at all.

[fiatjaf]

That's true. It's broken indeed. I'll try to fix it soon.

[dearchap]

How many utilities you see provide 3-4 level subcmds ? This library has to support that use case so we cannot make choices assuming a certain depth. urfave/cli is more akin to a platform than a cli library. It allows lots of configurations so that people can achieve what they want from a cli library. Just because something is good for one user doesnt mean that others want it. Any changes have to be backward compatible(within major versions) and if it is an optional feature it needs to turned off by default. Interested users should be able to enable it as needed. I have thought about this in the past and felt there needs to be a "personality" setting for the library which configures a set of defaults automatically to provide a given behavior. As an example we would provide say 'ls' personality or 'git' personality and the cli would behave similar to what those utilities do in terms of flags/subcmds in terms of how options are parsed(reorder args) and help output. Additionally users would be able to plugin their own personality engine which would define behaviors expected of the platform.

[fiatjaf]

I made it work for subcommands -- now it seems to be working fine in all my manual tests -- and made the behavior optional.

But some test in the suite and I'm having trouble understanding which. When I run go test I get a thousand lines of output from command calls. Please help me!

[dearchap]

@fiatjaf Can you add some test cases for this first ? If you want to test locally you can do

go test -v -failfast

that should give you test which failed immediately

[fiatjaf]

Working on tests now.

[dearchap]

@fiatjaf One more thing is to check the behavior when ShellCompletion is enabled. Pressing TAB and having the reorder, now sure how that might work.

[decentral1se]

One thing I ran into on manual testing latest changes. One example to compare from another tool. You can do hugo -h and hugo -h serve and still get the same main command help output. In the current changes, the -h would get reordered to hugo serve -h which I think will cause confusion. I think this might relate to the discussion in https://github.com/urfave/cli/issues/1950 where reordering should be "local first"?

[fiatjaf]

OK, I'm giving up, sorry.

[dearchap]

@fiatjaf if you'd like us to add this feature please keep the branch open. We can try picking it up later . As you can see it's not that trivial to implement . For a particular use case it's easy but to have it work for all users for different use cases is extremely difficult. That's something we've always struggled with

[abitrolly]

We can add "Unsolvable challenges" chapter with examples and clarification to documentation. That will be useful for people from all CLI frameworks to tackle this problem without spending extra too much time on researching these tangled bits.

[fiatjaf]

This "unsolvable challenge" is implemented in every other CLI framework, it's the number 1 requested feature according to your own link above, and this PR implements it perfectly as far as I know (except for the autocomplete thing which I have no idea of how it works, never managed to get it working on my apps).

Honestly, I don't understand why you're so dismissive of such a good feature that everybody wants. It would be a good idea to even sacrifice some of the other features just to get this, but I don't even think that is necessary. I just hope you take a look at it at some point in the future.

[abitrolly]

@fiatjaf hey, I am not dismissive. Now that you've created the issue, the link https://github.com/urfave/cli/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc correctly lists it as the most upvoted feature (with my upvote included), but before that there was no data to back up you claim. )

I can confirm that in Python argparse flags can come after arguments by default.

$ cat postarg.py
import argparse

parser = argparse.ArgumentParser()

parser.add_argument('filename')           # positional argument
parser.add_argument('-c', '--count')      # option that takes a value
parser.add_argument('-v', '--verbose',
                    action='store_true')  # on/off flag

args = parser.parse_args()
print(args.filename, args.count, args.verbose)

$ python postarg.py -v filename -c 4
filename 4 True
$ python postarg.py -c4 -v filename 
filename 4 True 

I personally ~~hate~~ don't like that go binary itself has very awful command help interface.

$ go mod -h
go mod: unknown command
Run 'go help mod' for usage. 

It could be much better if go supported -h option, which could be stuffed at the end.

[ellistarn]

👍 for this feature!

I decided to try out urfave/cli, as it had some nice idiomatic patterns compared to cobra. The lack of this feature is a deciding factor that will keep me using cobra. I will always favor the ergonomics of my customers rather than my own ergonomics as a developer.

[dearchap]

@fiatjaf I've implemented this feature in #1977 so as to keep backward compatibility and work with autocomplete. I am closing this. Can you review that PR ? Much appreciated

[dearchap]

@fiatjaf I have a potential fix at github.com:dearchap/cli/pos_args_flags_mix . Want to try it out ?