RFR: 1275: An unknown sub-command gives a confusing error message

[Erik Joelsson]

On Wed, 8 Dec 2021 14:17:51 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
>
> When using the unknown command, the client will output the unexpected message. For example:
>
>
> $ git-pr lis 123
>
> error: unexpected input: 123
> usage: git-pr [options] [<COMMAND>]
> 	-h, --help      Show help
> 	    --verbose   Turn on verbose output
> 	    --debug     Turn on debugging output
> 	    --version   Print the version of this tool
>
>
> When the client can't identify the sub-command `git-pr lis`, it will execute the default command `git-pr help` and will pass `lis 123` as the argument to the `git-pr help` command. And the `git-pr help` can resolve only one argument, which is `lis` in this example, so it would output `error: unexpected input: 123`.
>
> This patch will output more information to the user:
>
>
> $ git-pr lis 123
>
> Can't find the sub-command 'lis', the default sub-command 'help' will be executed.
> The arguments [lis, 123] will be passed to the default sub-command.
>
> error: unexpected input: 123
> usage: git-pr [options] [<COMMAND>]
> 	-h, --help      Show help
> 	    --verbose   Turn on verbose output
> 	    --debug     Turn on debugging output
> 	    --version   Print the version of this tool
>
>
> The added information can let the user know why the following output is shown.
>
> Thanks for taking the time to review.
>
> Best Regards,
> -- Guoxiong

The new message does explain in great detail how the implementation interprets the given input, but I think it's unnecessarily verbose. I think something like this would be clear enough:


$ git-pr lis 123

error: unknown sub-command: lis
error: unexpected input: 123
usage: git-pr [options] [<COMMAND>]
-h, --help      Show help
--verbose   Turn on verbose output
--debug     Turn on debugging output
--version   Print the version of this tool


However, not all users of MultiCommandParser have the help command as default. The webrev command does not and printing all of this text when running `webrev` with no explicit sub-command would be a regression IMO. Printing this error would need to be made conditional on if a sub-command is required or not. For `pr` it is, while for `webrev` it's not.

-------------

PR: https://git.openjdk.java.net/skara/pull/1257