doc/git-bisect: clarify `git bisect run` syntax

From: Javier Mora <[email protected]>

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.

---
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(-)

index 7872dba3aef..19bbed49238 100644
@@ -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

Okay, makes sense.

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

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.

Seems reasonable. I was actually wondering about that.

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

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.

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/[email protected]/

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.

Correct.

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.

But not as a part of this isolated fix.

From: Javier Mora <[email protected]>

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.

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

## 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(-)

index 7872dba3aef..191b4a42b6d 100644
@@ -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

index 65478ef40f5..35938b05fd1 100644
@@ -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

Makes sense to fix this inconsistency, as well, though 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").

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

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?

Yup.

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

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.

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