Cli: v2 bug: OnUsageError does not apply to "Incorrect Usage."

Created on 18 Dec 2019 · 14Comments · Source: urfave/cli

I removed the template because this is unnecessary in this case. I will just reference the lines with the bug (or lack of feature): https://github.com/urfave/cli/blob/master/app.go#L253 and https://github.com/urfave/cli/blob/master/app.go#L384.

Was this intended to be covered by OnUsageError? Right now when you pass invalid flags, you get this error message that you cannot customize in an easy way. The "workaround" for now is setting app.HideHelp to true and implementing your own (make sure to add your own OnUsageError field), or alternatively (bad idea for this particular issue), implement your own io.Writer and set app.Writer to it.

If it was not a bug because it was not supposed to be covered by OnUsageError, then you should implement FlagNotFound that works similarly to CommandNotFound because I see that you aim for customizability but it is an issue that you cannot change the error message you get when you pass unknown flags.

I am not quite sure why it does not work though as it does seem to call a.OnUsageError and returns after that, so theoretically it should not print it when OnUsageError is declared by me, right? I also noticed that despite having set both app.Name and app.HelpName, it is empty for go help -nonexistent. It works for other commands, as in it displays the app.Name properly for any other commands.

Okay, so I have been messing around with it. OnUsageError inside &cli.App but outside &cli.Flags and &cli.Commands does get executed, i.e. ./cli -foo help will run the function, but ./cli help -foo will not. I think the only option here to customize the error message for those sorts of usage errors is implementing your own help command. Is that correct? In any case, I did do just that and ~~it works perfectly~~. Nevermind, I have no way of knowing in OnUsageError which flag was the incorrect one among multiple ones. Any ideas?

arev2 help wanted kinbug statuconfirmed statustale

Source

odiferousmint

👀1

Most helpful comment

Hmm, to sum it up: I want to change the default Invalid Usage: ... text. Possibly using the same way CommandNotFound is implemented, except for flags. Alternatively you could expose the incorrect flag(s) that was/were given in OnUsageError which would suffice for me to be able to implement the help command on my end. No need to make it into a parameter of CommandNotFound, an exported variable will do, ideally with the type []string instead of []cli.Flag.

So either way works: 1) make FlagNotFound (or OptionNotFound), or 2) expose a list of incorrect flags given (as type []string) which could then be used in OnUsageError.

To give you an idea of what I am talking about:

$ ./cli -x help 
flag provided but not defined: -x
$ ./cli help -x
Incorrect Usage: flag provided but not defined: -x

NAME:
    - Shows a list of commands or help for one command

USAGE:
    [command options] [command]

OPTIONS:
   --help, -h  show help (default: false)

$

It seem like it is inconsistent as well (which could be considered an issue in itself), but I want to change:

flag provided but not defined: -x

and

Incorrect Usage: flag provided but not defined: -x

Personally I want to achieve this:

$ ./cli -x help 
cli: invalid option -- 'x'
Run 'cli help' for more information.
$ ./cli help -x
cli help: invalid option -- 'x'
Run 'cli help help' for more information.
$

Note the cli: ... part and the cli help help part! cli help will not give us the options for help, it only gives us the commands and the global options, but per command options are given via cli help <command>, so in this case it would be go help help. As far as cli: ... is concerned, it looks more reasonable given that the -x is supposed to be a global option and not one of cli help's option.

I suppose we could make an exception for help but the usage of cli help is cli help [command options] [command] so it make sense, although for help it looks a bit silly, but then how else would we know that command's options? Definitely not through cli help for the aforementioned reasons.

odiferousmint on 18 Dec 2019

🚀1 👍1

All 14 comments

@odiferousmint I would appreciate if you had kept the template, because I'm having a hard time identifying the concrete thing you're asking for 😅

lynncyrin on 18 Dec 2019

👍1

So either way works: 1) make FlagNotFound (or OptionNotFound), or 2) expose a list of incorrect flags given (as type []string) which could then be used in OnUsageError.

To give you an idea of what I am talking about:

$ ./cli -x help 
flag provided but not defined: -x
$ ./cli help -x
Incorrect Usage: flag provided but not defined: -x

NAME:
    - Shows a list of commands or help for one command

USAGE:
    [command options] [command]

OPTIONS:
   --help, -h  show help (default: false)

$

It seem like it is inconsistent as well (which could be considered an issue in itself), but I want to change:

flag provided but not defined: -x

and

Incorrect Usage: flag provided but not defined: -x

Personally I want to achieve this:

$ ./cli -x help 
cli: invalid option -- 'x'
Run 'cli help' for more information.
$ ./cli help -x
cli help: invalid option -- 'x'
Run 'cli help help' for more information.
$

odiferousmint on 18 Dec 2019

🚀1 👍1

I get it now 👍

lynncyrin on 18 Dec 2019

👍1

I edited my comment A LOT of times, so it is a bit more complicated again. :D I might end up implementing it myself because it is otherwise the best library for command-line parsing. The rest had even worse, major issues.

odiferousmint on 18 Dec 2019

👍1

@odiferousmint I am 100% in favor of you fixing this yourself! I can assign this issue to you, and the expectation would be that you work on it within the next ~2 months.

lynncyrin on 19 Dec 2019

ah to clarify, would you be interested in me assigning this issue to you @odiferousmint?

lynncyrin on 19 Dec 2019

I am extremely busy; traveling, holidays and such but I am already working on it. It seems to work so far as intended but it requires extensive testing with all possible combinations, and of course I will have to polish it up a bit. I do not really want to use GitHub for more than posting comments and I do not intend to submit a pull request here, I will do it via e-mail (I am old school! :D) if that is OK with you. It is going to take some time anyways. It will be alright, we will talk about this and other details when we get there. :)

Let us keep this issue open for the time being. I will change the title and we will close it when appropriate. Feel free to assign it to me, if for anything, for signalling that it is not an issue anyone else has to deal with.

(I fixed a completely unrelated thing, too, and I have a feeling I will have some of those commits as well. You/we will be able to cherry pick later. :D)

Edit: Yeah, I keep finding some obscure bugs. I will get to the bottom of this!

odiferousmint on 19 Dec 2019

👍1

FYI for whenever you read this -- I consider this issue claimed for 90 days from whenever you last commented / made other visible updates. So feel free to circle back anytime in the next _(glances)_ 82 days 😄

lynncyrin on 27 Dec 2019

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.