On 03-06-2020 01:15, Christian Couder wrote:
> On Tue, Jun 2, 2020 at 9:01 PM Kaartic Sivaraam
> <kaartic.sivaraam@xxxxxxxxx> wrote:
>>
>> I'm having second thoughts about my suggestion[1] to include
>> the short option for '--quiet' in the usage. This is the only
>> usage in submodule--helper that mentions that '-q' is a short
>> hand for '--quiet'. That seems inconsistent. I see two ways but
>> I'm not sure which one of these would be better:
>>
>> A. Dropping the mention of '-q' in this usage thus making it consistent
>> with the other usages printed by submodule--helper.
>>
>> B. Fixing other usages of submodule--helper to mention that '-q' is
>> shorthand for quiet. This has the benefit of properly advertising
>> the shorthand.
>>
>> C. Just ignore this?
>
> The `git submodule` documentation has:
>
> -q::
> --quiet::
> Only print error messages.
>
> even though the Synopsis is:
>
> 'git submodule' [--quiet] [--cached]
> 'git submodule' [--quiet] add [<options>] [--] <repository> [<path>]
> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> ...
>
> So I prefer B, and maybe updating the synopsis, as I think most Git
> commands have '-q' meaning '--quiet'.
>
Makes sense.
> [...]
>
>> So, according to this, I think the usage should be ...
>>
>> git submodule--helper set-branch [-q | --quiet] [-d | --default] <path>
>>
>> ... and ...
>>
>> git submodule--helper set-branch [-q|--quiet] [-b | --branch]<branch> <path>
>>
>> ... respectively.
>
> I don't agree. I think `git submodule--helper set-branch ...` requires
> either "-d | --default" or "-b | --branch", while for example:
>
> git submodule--helper set-branch [-q | --quiet] [-d | --default] <path>
>
> would mean that "git submodule--helper set-branch my/path" is valid.
>
You're right. Even I thought about the same thing when I came up with
that suggestion after quoting that portion of the CodingGuidelines. But
it was also curious for me to observe that the original used parenthesis
to mention the short and long options of an argument:
> + const char *const usage[] = {
> + N_("git submodule--helper set-branch [-q|--quiet] (-d|--default) <path>"),
> + N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
> + NULL
> + };
I've not seen such a usage before. That's what actually made me take a
look at the CodingGuidelines for this. As the CodingGuidelined doesn't
seem to be mentioning anything about this explicitly, let's see if I
could find something in the usage printed by other commands.
---
> git am -h
usage: git am [<options>] [(<mbox> | <Maildir>)...]
or: git am [<options>] (--continue | --skip | --abort)
<options snipped>
> git branch -h
usage: git branch [<options>] [-r | -a] [--merged | --no-merged]
or: git branch [<options>] [-l] [-f] <branch-name> [<start-point>]
or: git branch [<options>] [-r] (-d | -D) <branch-name>...
or: git branch [<options>] (-m | -M) [<old-branch>] <new-branch>
or: git branch [<options>] (-c | -C) [<old-branch>] <new-branch>
or: git branch [<options>] [-r | -a] [--points-at]
or: git branch [<options>] [-r | -a] [--format]
<options snipped>
> git checkout -h
usage: git checkout [<options>] <branch>
or: git checkout [<options>] [<branch>] -- <file>...
<options snipped>
> git switch -h
usage: git switch [<options>] [<branch>]
<options snipped>
---
Hmm. Looks like it's not common for us to mention both the short
and long options in the usage itself. This might be to avoid the
redundancy as the usage is usually followed by the list of options.
With this info, I think we could've just gone with the following as the
usage strings for the `set-branch` subcommand:
git submodule--helper set-branch [<options>] -d <path>
git submodule--helper set-branch [<options>] -b <branch> <path>
This also solves the problem with `--quiet` I mentioned earlier while
making it concise and inline with the usages printed by other commands.
All this said, I don't think it's worth a re-roll now for several
reasons.
--
Sivaraam
