Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 23, 2023, 19:36 UTC
Message-ID: <CAPig+cQuBwzaG7ZssGUY6k8wf8pcGZHAGLnbRy579uTPMKqwKQ@mail.gmail.com>
In-Reply-To: <pull.1602.v2.git.1698088990478.gitgitgadget@gmail.com>

On Mon, Oct 23, 2023 at 3:23 PM cousteau via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 14 quoted lines

> doc/git-bisect: clarify `git bisect run` syntax
>
> The description of the `git bisect run` command syntax at the beginning
> of the manpage is `git bisect run <cmd>...`, which isn't quite clear
> about what `<cmd>` is or what the `...` mean; one could think that it is
> the whole (quoted) command line with all arguments in a single string,
> or that it supports multiple commands, or that it doesn't accept
> commands with arguments at all.
>
> Change to `git bisect run <cmd> [<arg>...]` to clarify the syntax,
> in both the manpage and the `git bisect -h` command output.
>
> Additionally, change `--term-{new,bad}` et al to `--term-(new|bad)`
> for consistency with the synopsis syntax conventions.

Makes sense to fix this inconsistency, as well, though the patch subject becomes a bit outdated with this addition.

Show 9 quoted lines

> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> ---
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -16,7 +16,7 @@ DESCRIPTION
> - git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
> + git bisect start [--term-(new|bad)=<term-new> --term-(old|good)=<term-old>]
>                   [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
>   git bisect (bad|new|<term-new>) [<rev>]
>   git bisect (good|old|<term-old>) [<rev>...]

Upon first reading, I questioned whether changing <term> to <term-new> and <term-old> adds value since the option names --term-new and --term-old already provide enough context for the reader to understand the generic placeholder <term>. However, then I noticed that the following two lines are already referencing placeholders <term-new> and <term-old>, so perhaps this change makes sense. But...

Show 5 quoted lines

> diff --git a/builtin/bisect.c b/builtin/bisect.c
> @@ -26,7 +26,7 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
>  #define BUILTIN_GIT_BISECT_START_USAGE \
> -       N_("git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]" \
> +       N_("git bisect start [--term-(new|bad)=<term> --term-(old|good)=<term>]" \

...now we have an inconsistency again since this text just uses the generic <term>. However, I haven't convinced myself that we need to care about this inconsistency.