click icon indicating copy to clipboard operation
click copied to clipboard

Command w/o args displaying usage should return non-zero

Open benjaoming opened this issue 6 years ago • 10 comments

Try running git, and you will see its usage. You will also receive a non-zero return code, indicating failure. This can be vital in case arguments got truncated.

Current behavior of showing usage instructions (help_text) is great!

But it should also error out with a code 1.

Example:

➜  git
usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
           [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]
           [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
           <command> [<args>]

(snip)

'git help -a' and 'git help -g' list available subcommands and some
concept guides. See 'git help <command>' or 'git help <concept>'
to read about a specific subcommand or concept.
➜  echo $?
1

Reference: https://github.com/learningequality/kolibri/pull/5937

benjaoming avatar Sep 13 '19 13:09 benjaoming

I'm not completely understanding, can you clarify what you are asking for?

Non zero exit codes are returned in some cases, if there is a missing argument or option. Git actually returns a 0 exit code if you call it with --help.

jcrotts avatar Sep 13 '19 17:09 jcrotts

I'm not completely understanding, can you clarify what you are asking for?

If my wonderful-command is run without arguments, it should be understood as an error. But it should also output usage instructions.

Git actually returns a 0 exit code if you call it with --help.

That's because git --help is understood as a deliberate command. git is understood as a wrong command but will output usage instructions for convenience.

benjaoming avatar Sep 13 '19 18:09 benjaoming

I looked at the linked issue, what specifically would you change within click.

The following seems fairly straightforward to add if your Click app doesn't throw a missing argument exception, but you still want it to return a non zero exit code.

    if ctx.invoked_subcommand is None:
        click.echo(ctx.get_help())
        ctx.exit(1)

jcrotts avatar Sep 14 '19 23:09 jcrotts

Yes, it's simple to implement, as you can see that's what we did -- but perhaps Click can add it as default behavior? I intended to open the issue to flag that it seems like a deviation from conventional behavior.

benjaoming avatar Sep 16 '19 15:09 benjaoming

You've only described it differing from git's behavior, not "conventional" behavior. Is there any convention or standard for this that is demonstrated by a majority of applications?

I don't see "no args is error" as any more valid than "no args printing help is expected, so not an error", so I don't currently see a compelling reason to change this.

davidism avatar Sep 16 '19 16:09 davidism

@davidism

so I don't currently see a compelling reason to change this

mycommand $filename <= if $filename is empty by mistake, then it will return 0 as if execution was successful.

benjaoming avatar Sep 16 '19 19:09 benjaoming

Is there any convention or standard for this that is demonstrated by a majority of applications?

I tested git, rsync, ssh, cp, mv, rm, ip. They all return non-zero.

benjaoming avatar Sep 16 '19 19:09 benjaoming

This only happens with groups - invoking a (sub-)command requiring arguments, without arguments, will raise and exit with a non-zero status code. The default behaviour of a group is not to call the callback (i.e. the wrapped function) if the argument is not supplied, which presupposes that it is required. Click should exit with a non-zero status code if invoke_without_command is false and defer to the callback if it's true.

layday avatar Oct 07 '19 13:10 layday

If anyone else has trouble parsing @benjaoming's comment above, the "<=" is not meant to be "less than or equal". It's meant more like "so" or just full stop. In other words:

mycommand $filename. If $filename is empty by mistake, then [Click's current default behavior is to] return 0 as if execution was successful.

I do see this as one valid reason that Click's current default behavior might lead to unexpected results.

I also have seen, more often than not, widely-used command line tools print usage to stderr and exit nonzero when invoked without required arg(s).

But while changing this default in Click may help for this use case, it's not immediately obvious that it wouldn't hurt other use cases, given the wide variety of different CLIs this could affect. It would be helpful if someone could show how this change would affect other use cases.

jab avatar Jul 12 '20 15:07 jab

Thanks @jab , here is the example (from https://github.com/pallets/click/issues/1394#issuecomment-531923934) of unintentional behaviour again:

# if `$filename` is empty by mistake, then the command will return 0 as if execution was successful.
# But this should really be a wrong call that returns a non-zero exit code.
mycommand $filename

benjaoming avatar Jul 22 '20 10:07 benjaoming