Re: [RFC/PATCH 1/2] submodule: implement `module_list` as a builtin helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> On Fri, Jul 31, 2015 at 6:01 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>>
>>> +static const char * const git_submodule_helper_usage[] = {
>>> +     N_("git submodule--helper --module_list [<path>...]"),
>>
>> Yuck.  Please do not force --multi_word_opt upon us, which is simply
>> too ugly to live around here.  --module-list is perhaps OK,
>
> I agree there. The way you word it here, it sounds as if the mixture
> of dashes and underscores are a problem.
>
>> but
>> because submodule--helper would not have an default action, I'd
>> prefer to make these just "command words", i.e.
>>
>>     $ git submodule--helper module_list
>
> Why would you use an underscore in here as opposed to a dash?
>      $ git submodule--helper module-list
>
> I went with --module-list for now as I see no reason real to make it
> a command word for now ...

The biggest reason why we should not add more --command-mode is to
avoid confusion (and copy & paste misdesign by others).  If you use
the command-word interface, it is crystal clear that

 (1) the word 'module_list' must be the first token after the
     subcommand name, no need to parse "subcmd --opt --cmd", and
     mislead the users to think incorrectly that ...

 (2) ... "cmd --optA --cmd1 --optB --cmd2" might be allowed, which
     would lead you to add code to reject, saying "cmd1 and cmd2 are
     incompatible".

So I'd prefer to see it fixed before you start supporting more
commands in submodule--helper.  It will need unnecessary patch noise
to fix it later.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]