cli: OnUsageError does not apply to "Incorrect Usage."

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?

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 1
Comments: 15 (7 by maintainers)

Most upvoted comments

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 Dec 18, 2019

Bumping, have same issue

tpihl on Mar 27, 2020

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! 😄) 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. 😄)

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

odiferousmint on Dec 19, 2019

I edited my comment A LOT of times, so it is a bit more complicated again. 😄 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 Dec 18, 2019

I get it now 👍

coilysiren on Dec 18, 2019

@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 😅

coilysiren on Dec 18, 2019