Re: [PATCH 2/3] submodule: implement `module_name` as a builtin helper

[Stefan Beller <sbeller@google.com>]

On Mon, Aug 31, 2015 at 1:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>  usage:
>> -     usage("git submodule--helper module_list\n");
>> +     usage("git submodule--helper [module_list | module_name]\n");
>
> To me, the above reads as if saying:
>
>     The command takes one of the two subcommands at this stage,
>     module_list that does not take any parameter, and module_name
>     that does not take any parameter.
>
> which is not what you intended.
>
> I think that the help for individual options and arguments are
> sufficiently given in the implementation of each subcommand
> (e.g. module_list does its own parse_options() thing), so there is
> no need to duplicate them here.  The only purpose of this usage serves
> is to tell the user that the subcommand name was not understood, and
> give the list of available subcommands.  For that, I wonder if the
> usual single-liner "usage" is the best way to do so.
>
>     $ git submodule--helper frotz
>     fatal: 'frotz' is not a valid submodule--helper subcommand, which are
>            module_list, module_name.
>
> or something along that line, perhaps, may be more appropriate?

As this is something that *should* not happen in the wild, (but it
will of course),
it sounds like a good idea to have a clear error message here. I'll send a patch
for that. (either one on top of 3/3 to improve the message, or a
reroll of the series,
as you'd like)