git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] doc/git-bisect: clarify `git bisect run` syntax
@ 2023-10-22 20:02 cousteau via GitGitGadget
  2023-10-22 21:32 ` Eric Sunshine
  2023-10-23 19:23 ` [PATCH v2] " cousteau via GitGitGadget
  0 siblings, 2 replies; 12+ messages in thread
From: cousteau via GitGitGadget @ 2023-10-22 20:02 UTC (permalink / raw)
  To: git; +Cc: cousteau, Javier Mora

From: Javier Mora <cousteaulecommandant@gmail.com>

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.

Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
---
    doc/git-bisect: clarify git bisect run syntax
    
    I saw someone in IRC wondering about the syntax for git bisect run for a
    command with arguments, and found that its short description at the
    beginning of the manpage is not very clear (although it gets clarified
    later when it is properly described). It describes the syntax as git
    bisect run <cmd>... which is a bit confusing; it should say git bisect
    run <cmd> [<arg>...], otherwise it somehow looks like you have to "enter
    one or more commands", and that each command is a single argument.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1602%2Fcousteaulecommandant%2Fman-git-bisect-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1602/cousteaulecommandant/man-git-bisect-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1602

 Documentation/git-bisect.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 7872dba3aef..19bbed49238 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -26,7 +26,7 @@ on the subcommand:
  git bisect (visualize|view)
  git bisect replay <logfile>
  git bisect log
- git bisect run <cmd>...
+ git bisect run <cmd> [<arg>...]
  git bisect help
 
 This command uses a binary search algorithm to find which commit in

base-commit: ceadf0f3cf51550166a387ec8508bb55e7883057
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-22 20:02 [PATCH] doc/git-bisect: clarify `git bisect run` syntax cousteau via GitGitGadget
@ 2023-10-22 21:32 ` Eric Sunshine
  2023-10-23  0:35   ` Junio C Hamano
  2023-10-23 19:23 ` [PATCH v2] " cousteau via GitGitGadget
  1 sibling, 1 reply; 12+ messages in thread
From: Eric Sunshine @ 2023-10-22 21:32 UTC (permalink / raw)
  To: cousteau via GitGitGadget; +Cc: git, Javier Mora

On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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.

Okay, makes sense.

> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> ---
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -26,7 +26,7 @@ on the subcommand:
> - git bisect run <cmd>...
> + git bisect run <cmd> [<arg>...]

The output of `git bisect -h` suffers the same problem. Perhaps this
patch can fix that, as well?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-22 21:32 ` Eric Sunshine
@ 2023-10-23  0:35   ` Junio C Hamano
  2023-10-23  7:38     ` Patrick Steinhardt
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2023-10-23  0:35 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: cousteau via GitGitGadget, git, Javier Mora

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> 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.
>
> Okay, makes sense.
>
>> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
>> ---
>> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
>> @@ -26,7 +26,7 @@ on the subcommand:
>> - git bisect run <cmd>...
>> + git bisect run <cmd> [<arg>...]
>
> The output of `git bisect -h` suffers the same problem. Perhaps this
> patch can fix that, as well?

Good eyes.

Not a new problem and obviously can be left outside of this simple
update, but I wonder if we should eventually move these into the
proper SYNOPSIS section.  Other multi-modal commands like "git
checkout", "git rebase", etc. do list different forms all in the
SYNOPSIS section.

I also thought at least some commands we know the "-h" output and
SYNOPSIS match, we had tests to ensure they do not drift apart.  We
would probably want to cover more subcommands with t0450.

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23  0:35   ` Junio C Hamano
@ 2023-10-23  7:38     ` Patrick Steinhardt
  2023-10-23 16:27       ` Javier Mora
  2023-10-23 17:23       ` Junio C Hamano
  0 siblings, 2 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2023-10-23  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, cousteau via GitGitGadget, git, Javier Mora

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

On Sun, Oct 22, 2023 at 05:35:41PM -0700, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >> 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.
> >
> > Okay, makes sense.
> >
> >> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> >> ---
> >> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> >> @@ -26,7 +26,7 @@ on the subcommand:
> >> - git bisect run <cmd>...
> >> + git bisect run <cmd> [<arg>...]
> >
> > The output of `git bisect -h` suffers the same problem. Perhaps this
> > patch can fix that, as well?
> 
> Good eyes.
> 
> Not a new problem and obviously can be left outside of this simple
> update, but I wonder if we should eventually move these into the
> proper SYNOPSIS section.  Other multi-modal commands like "git
> checkout", "git rebase", etc. do list different forms all in the
> SYNOPSIS section.
> 
> I also thought at least some commands we know the "-h" output and
> SYNOPSIS match, we had tests to ensure they do not drift apart.  We
> would probably want to cover more subcommands with t0450.
> 
> Thanks.

If we don't want them to drift apart I wonder whether we could instead
generate the synopsis from the output of `-h`? This reduces duplication
at the cost of a more complex build process for our manpages.

Not saying that this is necessarily a good idea, just throwing it out
there.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23  7:38     ` Patrick Steinhardt
@ 2023-10-23 16:27       ` Javier Mora
  2023-10-23 17:46         ` Junio C Hamano
  2023-10-23 17:23       ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Javier Mora @ 2023-10-23 16:27 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Junio C Hamano, Eric Sunshine, cousteau via GitGitGadget, git

> The output of `git bisect -h` suffers the same problem. Perhaps this
> patch can fix that, as well?

Certainly possible.  Probably best if I put that on a second patch
though (i.e. a separate commit).  Or should I just squash everything
together?

There are still multiple .po files containing the old string, I guess
I don't need to touch those?

Speaking of which, looking at the .po files I've found that there's
also a `git bisect--helper` command; I don't know if that's relevant
nor how to modify that.

> I wonder if we should eventually move these into the
> proper SYNOPSIS section.

Seems reasonable.  I was actually wondering about that.

I can make an extra patch for that if you want, while I'm at it.

> If we don't want them to drift apart I wonder whether we could instead
> generate the synopsis from the output of `-h`? This reduces duplication

That's not a bad idea.  Or maybe the other way around -- generate the
output of `-h` from the synopsis.  Or generate both (manpage and help
message) from a "synopsis stub" file; I wonder if that could be easily
done.


El lun, 23 oct 2023 a las 8:38, Patrick Steinhardt (<ps@pks.im>) escribió:
>
> On Sun, Oct 22, 2023 at 05:35:41PM -0700, Junio C Hamano wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> >
> > > On Sun, Oct 22, 2023 at 4:03 PM cousteau via GitGitGadget
> > > <gitgitgadget@gmail.com> wrote:
> > >> 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.
> > >
> > > Okay, makes sense.
> > >
> > >> Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
> > >> ---
> > >> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> > >> @@ -26,7 +26,7 @@ on the subcommand:
> > >> - git bisect run <cmd>...
> > >> + git bisect run <cmd> [<arg>...]
> > >
> > > The output of `git bisect -h` suffers the same problem. Perhaps this
> > > patch can fix that, as well?
> >
> > Good eyes.
> >
> > Not a new problem and obviously can be left outside of this simple
> > update, but I wonder if we should eventually move these into the
> > proper SYNOPSIS section.  Other multi-modal commands like "git
> > checkout", "git rebase", etc. do list different forms all in the
> > SYNOPSIS section.
> >
> > I also thought at least some commands we know the "-h" output and
> > SYNOPSIS match, we had tests to ensure they do not drift apart.  We
> > would probably want to cover more subcommands with t0450.
> >
> > Thanks.
>
> If we don't want them to drift apart I wonder whether we could instead
> generate the synopsis from the output of `-h`? This reduces duplication
> at the cost of a more complex build process for our manpages.
>
> Not saying that this is necessarily a good idea, just throwing it out
> there.
>
> Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23  7:38     ` Patrick Steinhardt
  2023-10-23 16:27       ` Javier Mora
@ 2023-10-23 17:23       ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2023-10-23 17:23 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Eric Sunshine, cousteau via GitGitGadget, git, Javier Mora

Patrick Steinhardt <ps@pks.im> writes:

>> I also thought at least some commands we know the "-h" output and
>> SYNOPSIS match, we had tests to ensure they do not drift apart.  We
>> would probably want to cover more subcommands with t0450.
>> 
>> Thanks.
>
> If we don't want them to drift apart I wonder whether we could instead
> generate the synopsis from the output of `-h`? This reduces duplication
> at the cost of a more complex build process for our manpages.

There also is the cost of making it unusable to peek the source
git-foo.txt files as a quick way to get the usage guide, which I
think is a downside only felt by developers of Git (not developers
of other projects that happen to use Git), but still a downside.

But aside from that, it is an obviously possible direction to go [*]
and in fact I suspect we may have talked about it when Ævar made a
gigantic effort to clean these up in Sep-Oct 2022 timeframe, which
resulted in the series leading to a0343f30 (tests: assert consistent
whitespace in -h output, 2022-10-13) [*].

[Footnote]

 * And another possibility is to go from the doc to the message
   string, which may be even more involved, but at least the code
   needs to go through the build process anyway, so the downside
   might be lessor)

 * https://lore.kernel.org/git/patch-v5-34.34-4de83d3d89a-20221013T153626Z-avarab@gmail.com/



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23 16:27       ` Javier Mora
@ 2023-10-23 17:46         ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2023-10-23 17:46 UTC (permalink / raw)
  To: Javier Mora
  Cc: Patrick Steinhardt, Eric Sunshine, cousteau via GitGitGadget, git

Javier Mora <cousteaulecommandant@gmail.com> writes:

>> The output of `git bisect -h` suffers the same problem. Perhaps this
>> patch can fix that, as well?
>
> Certainly possible.  Probably best if I put that on a second patch
> though (i.e. a separate commit).  Or should I just squash everything
> together?

In this case, a single patch is the way to go; otherwise we will
(tentatively) be in an inconsistent state after applying one until
the other gets applied.

> There are still multiple .po files containing the old string, I guess
> I don't need to touch those?

Correct.

> Speaking of which, looking at the .po files I've found that there's
> also a `git bisect--helper` command; I don't know if that's relevant
> nor how to modify that.

bisect--helper has been retired but most of the messages used by it
should have been in use by bisect proper, so only the "this message
appears here" comments may be wrong.

In any case, touching po/ is not in the scope of this isolated fix.
The i18n group has their own workflows to update the files there,
and those touching the code and docs should not have to touch them
in general.

>> I wonder if we should eventually move these into the
>> proper SYNOPSIS section.
>
> Seems reasonable.  I was actually wondering about that.

But not as a part of this isolated fix.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-22 20:02 [PATCH] doc/git-bisect: clarify `git bisect run` syntax cousteau via GitGitGadget
  2023-10-22 21:32 ` Eric Sunshine
@ 2023-10-23 19:23 ` cousteau via GitGitGadget
  2023-10-23 19:36   ` Eric Sunshine
  1 sibling, 1 reply; 12+ messages in thread
From: cousteau via GitGitGadget @ 2023-10-23 19:23 UTC (permalink / raw)
  To: git; +Cc: Eric Sunshine, Patrick Steinhardt, Javier Mora, cousteau,
	Javier Mora

From: Javier Mora <cousteaulecommandant@gmail.com>

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.

Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
---
    doc/git-bisect: clarify git bisect run syntax
    
    I saw someone in IRC wondering about the syntax for git bisect run for a
    command with arguments, and found that its short description at the
    beginning of the manpage is not very clear (although it gets clarified
    later when it is properly described). It describes the syntax as git
    bisect run <cmd>... which is a bit confusing; it should say git bisect
    run <cmd> [<arg>...], otherwise it somehow looks like you have to "enter
    one or more commands", and that each command is a single argument.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1602%2Fcousteaulecommandant%2Fman-git-bisect-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1602/cousteaulecommandant/man-git-bisect-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1602

Range-diff vs v1:

 1:  ce4c60a6f4f ! 1:  8de70bb060e doc/git-bisect: clarify `git bisect run` syntax
     @@ Commit message
          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.
     +    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.
      
          Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
      
       ## Documentation/git-bisect.txt ##
     +@@ Documentation/git-bisect.txt: DESCRIPTION
     + The command takes various subcommands, and different options depending
     + on the subcommand:
     + 
     +- 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>...]
      @@ Documentation/git-bisect.txt: on the subcommand:
        git bisect (visualize|view)
        git bisect replay <logfile>
     @@ Documentation/git-bisect.txt: on the subcommand:
        git bisect help
       
       This command uses a binary search algorithm to find which commit in
     +
     + ## builtin/bisect.c ##
     +@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
     + static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
     + 
     + #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>]" \
     + 	   "    [--no-checkout] [--first-parent] [<bad> [<good>...]] [--]" \
     + 	   "    [<pathspec>...]")
     + #define BUILTIN_GIT_BISECT_STATE_USAGE \
     +@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
     + #define BUILTIN_GIT_BISECT_LOG_USAGE \
     + 	"git bisect log"
     + #define BUILTIN_GIT_BISECT_RUN_USAGE \
     +-	N_("git bisect run <cmd>...")
     ++	N_("git bisect run <cmd> [<arg>...]")
     + 
     + static const char * const git_bisect_usage[] = {
     + 	BUILTIN_GIT_BISECT_START_USAGE,


 Documentation/git-bisect.txt | 4 ++--
 builtin/bisect.c             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 7872dba3aef..191b4a42b6d 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -16,7 +16,7 @@ DESCRIPTION
 The command takes various subcommands, and different options depending
 on the subcommand:
 
- 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>...]
@@ -26,7 +26,7 @@ on the subcommand:
  git bisect (visualize|view)
  git bisect replay <logfile>
  git bisect log
- git bisect run <cmd>...
+ git bisect run <cmd> [<arg>...]
  git bisect help
 
 This command uses a binary search algorithm to find which commit in
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 65478ef40f5..35938b05fd1 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -26,7 +26,7 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 
 #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>]" \
 	   "    [--no-checkout] [--first-parent] [<bad> [<good>...]] [--]" \
 	   "    [<pathspec>...]")
 #define BUILTIN_GIT_BISECT_STATE_USAGE \
@@ -46,7 +46,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 #define BUILTIN_GIT_BISECT_LOG_USAGE \
 	"git bisect log"
 #define BUILTIN_GIT_BISECT_RUN_USAGE \
-	N_("git bisect run <cmd>...")
+	N_("git bisect run <cmd> [<arg>...]")
 
 static const char * const git_bisect_usage[] = {
 	BUILTIN_GIT_BISECT_START_USAGE,

base-commit: ceadf0f3cf51550166a387ec8508bb55e7883057
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23 19:23 ` [PATCH v2] " cousteau via GitGitGadget
@ 2023-10-23 19:36   ` Eric Sunshine
  2023-10-23 22:53     ` Javier Mora
  0 siblings, 1 reply; 12+ messages in thread
From: Eric Sunshine @ 2023-10-23 19:36 UTC (permalink / raw)
  To: cousteau via GitGitGadget; +Cc: git, Patrick Steinhardt, Javier Mora

On Mon, Oct 23, 2023 at 3:23 PM cousteau via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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.

> 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...

> 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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23 19:36   ` Eric Sunshine
@ 2023-10-23 22:53     ` Javier Mora
  2023-10-23 23:18       ` Eric Sunshine
  2023-10-24  0:12       ` Junio C Hamano
  0 siblings, 2 replies; 12+ messages in thread
From: Javier Mora @ 2023-10-23 22:53 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: cousteau via GitGitGadget, git, Patrick Steinhardt

> the patch subject becomes a bit outdated with this addition.

Right; I wanted to change it to something like "clarify `git bisect
run` syntax and other minor changes" but wanted to keep the title
concise.
I guess I could change it to just "clarify `git bisect` syntax" though
remove the "run").

> the following two lines are already referencing placeholders
> <term-new> and <term-old>

That's why I added it; that `(bad|new|<term-new>)` felt a bit awkward
with no previous explanation of what <term-new> was.

> ...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.

I thought about that, but in THAT case it wasn't necessary because
<term-new> and <term-old> are never used there (and I wanted to avoid
making -h too long).  But it's true that it feels inconsistent; I may
add it just for the sake of consistency.

Overall, maybe I should leave that change to a separate patch, even if
it's a minor correction.  (This made more sense when I had in mind the
plan to move everything from description to synopsis so I would need
to touch all those lines anyway.)  The changes will be compatible
anyway (they're far away enough to not cause merge conflicts).  What
do you think?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23 22:53     ` Javier Mora
@ 2023-10-23 23:18       ` Eric Sunshine
  2023-10-24  0:12       ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Eric Sunshine @ 2023-10-23 23:18 UTC (permalink / raw)
  To: Javier Mora; +Cc: cousteau via GitGitGadget, git, Patrick Steinhardt

On Mon, Oct 23, 2023 at 6:53 PM Javier Mora
<cousteaulecommandant@gmail.com> wrote:
> > the patch subject becomes a bit outdated with this addition.
>
> Right; I wanted to change it to something like "clarify `git bisect
> run` syntax and other minor changes" but wanted to keep the title
> concise.
> I guess I could change it to just "clarify `git bisect` syntax" though
> remove the "run").

Yup.

> > the following two lines are already referencing placeholders
> > <term-new> and <term-old>
>
> That's why I added it; that `(bad|new|<term-new>)` felt a bit awkward
> with no previous explanation of what <term-new> was.
>
> > ...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.
>
> I thought about that, but in THAT case it wasn't necessary because
> <term-new> and <term-old> are never used there (and I wanted to avoid
> making -h too long).  But it's true that it feels inconsistent; I may
> add it just for the sake of consistency.

I don't feel strongly about the inconsistency at this point.

> Overall, maybe I should leave that change to a separate patch, even if
> it's a minor correction.  (This made more sense when I had in mind the
> plan to move everything from description to synopsis so I would need
> to touch all those lines anyway.)  The changes will be compatible
> anyway (they're far away enough to not cause merge conflicts).  What
> do you think?

I can certainly see the "{new,bad}" to "(new|bad") and <term> to
<new-term>/<old-term> changes being separated out, making this a two-
or three-patch series.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
  2023-10-23 22:53     ` Javier Mora
  2023-10-23 23:18       ` Eric Sunshine
@ 2023-10-24  0:12       ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2023-10-24  0:12 UTC (permalink / raw)
  To: Javier Mora
  Cc: Eric Sunshine, cousteau via GitGitGadget, git, Patrick Steinhardt

Javier Mora <cousteaulecommandant@gmail.com> writes:

>> the patch subject becomes a bit outdated with this addition.
>
> Right; I wanted to change it to something like "clarify `git bisect
> run` syntax and other minor changes" but wanted to keep the title
> concise.
> I guess I could change it to just "clarify `git bisect` syntax" though
> remove the "run").

Quite honestly, I think at this point we are entering into the
"diminishing returns" territory.  The title is still clear enough,
and the patch is good.

Thanks.  The patch has been merged to 'next'.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-10-24  0:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-22 20:02 [PATCH] doc/git-bisect: clarify `git bisect run` syntax cousteau via GitGitGadget
2023-10-22 21:32 ` Eric Sunshine
2023-10-23  0:35   ` Junio C Hamano
2023-10-23  7:38     ` Patrick Steinhardt
2023-10-23 16:27       ` Javier Mora
2023-10-23 17:46         ` Junio C Hamano
2023-10-23 17:23       ` Junio C Hamano
2023-10-23 19:23 ` [PATCH v2] " cousteau via GitGitGadget
2023-10-23 19:36   ` Eric Sunshine
2023-10-23 22:53     ` Javier Mora
2023-10-23 23:18       ` Eric Sunshine
2023-10-24  0:12       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).