Subject: Re: [PATCH 00/34] doc/UX: make txt & -h output more consistent

From: Victoria Dye <[email-masked]>
Date: Mon, 19 Sep 2022 15:15:33 -0700

Ævar Arnfjörð Bjarmason wrote:
> We are currently wildly inconsistent in whether the SYNOPSIS in the
> manual page matches the first line of the -h output, and as we add new
> options we routinely forget to add them to one or the other (or both).
> 
> Without a more complex approach it's hard to do something about the
> "or both" case. But we can rather easily test whether the -h output
> matches the *.txt version, and report differences.
> 
> As this series shows that allows us to fix a lot of issues we've
> effectively already "fixed", we just fixed them in one version, but
> not the other.
> 
> We now have a test that checks that these are the same for all
> built-ins. Out of 141 built-in commands we still have 58 cases that
> differ at the end of this series, but that's a lot better than before.

Thanks! Codifying these expectations into an automated check will be
extremely helpful for avoiding easy-to-miss mistakes that could potentially
confuse end users.

> 
> This series is 34 patches, but it's been structured in a way that
> reviewers should be able to mostly trust the "doc txt & -h
> consistency" commits already. In all of those cases we already have
> the post-image in-tree, it's just in either the *.txt or -h version,
> not both. All of the "doc txt & -h consistency" commits merely make
> those consistent together.

In general, I don't think it's wise to implicitly trust *any* patches, no
matter how simple they appear. I know there are benefits to doing everything
at once, but the volume and variety of these changes makes it difficult to
review the series and *confidently* say we haven't introduced any
regressions, missed any cases, or made the "wrong" decisions w.r.t changing
either the .txt or -h documentation to make them consistent.

After reading through this series a few times, it looks like it contains a
mix of:

* updated syntax guidance in CodingGuidelines
* clear syntax or naming fixes (e.g., adding a missing "]" to a '-h' output,
  "keyid" -> "key-id" for translation purposes)
* internal refactoring
* changing an error message's content
* changing the listed options and/or usage in '-h' and/or the .txt SYNOPSIS
* adding an option description in a .txt doc
* adding the new test

If I could offer a suggestion, my preference would be that you split the
series into three parts: one with the straightforward, easier-to-review
changes, another with the more substantial updates to user-facing
docs/information (which might warrant more discussion, i.e. which options we
should be showing for a command in the SYNOPSIS/-h), and the last catching
any new inconsistencies & adding the test. That way, more involved
discussion on some patches doesn't prevent *all* of them from being merged.

I think the following patches would fit a "straightforward,
easier-to-review" series:

* Patch 1 (CodingGuidelines: update and clarify command-line conventions)
* Patch 2 (builtin/bundle.c: use \t, not fix indentation 2-SP indentation)
* Patch 3 (bundle: define subcommand -h in terms of command -h)
* Patch 5 (doc SYNOPSIS: don't use ' for subcommands)
* Patch 6 (doc SYNOPSIS: consistently use ' for commands)
* Patch 7 (doc SYNOPSIS & -h: fix incorrect alternates syntax)
* Patch 8 (built-ins: consistently add "\n" between "usage" and options)
* Patch 9 (doc txt & -h consistency: word-wrap)
* Patch 10 (doc txt & -h consistency: fix incorrect alternates syntax)
* Patch 12 (doc txt & -h consistency: add missing "]" to bugreport "-h")
* Patch 13 (doc txt & -h consistency: correct padding around "[]()")
* Patch 14 (stash doc SYNOPSIS & -h: correct padding around "[]()")
* Patch 15 (doc txt & -h consistency: use "<options>", not "<options>...")
* Patch 16 (t/helper/test-proc-receive.c: use "<options>", not
  "<options>...")
* Patch 17 (doc txt & -h consistency: fix mismatching labels)
* Patch 18 (doc txt & -h consistency: add or fix optional "--" syntax)

(basically, 1-18, skipping patch 4 because it changes the content of an
error message & patch 11 because it adds an option to the -h of 'cat-file')

In terms of review, my only comment on those patches is that 7 & 10 could
probably benefit from being squashed together [1]. Otherwise, with the
changes you mentioned in response to Junio's feedback [2], I think that
subset of the series would be ready to merge.

I hope that helps!
-Victoria

[1] https://lore.kernel.org/git/[email-masked]/
[2] https://lore.kernel.org/git/[email-masked]/

> 
> Ævar Arnfjörð Bjarmason (34):
>   CodingGuidelines: update and clarify command-line conventions
>   builtin/bundle.c: use \t, not fix indentation 2-SP indentation
>   bundle: define subcommand -h in terms of command -h
>   blame: use a more detailed usage_msg_optf() error on bad -L
>   doc SYNOPSIS: don't use ' for subcommands
>   doc SYNOPSIS: consistently use ' for commands
>   doc SYNOPSIS & -h: fix incorrect alternates syntax
>   built-ins: consistently add "\n" between "usage" and options
>   doc txt & -h consistency: word-wrap
>   doc txt & -h consistency: fix incorrect alternates syntax
>   doc txt & -h consistency: add "-z" to cat-file "-h"
>   doc txt & -h consistency: add missing "]" to bugreport "-h"
>   doc txt & -h consistency: correct padding around "[]()"
>   stash doc SYNOPSIS & -h: correct padding around "[]()"
>   doc txt & -h consistency: use "<options>", not "<options>..."
>   t/helper/test-proc-receive.c: use "<options>", not "<options>..."
>   doc txt & -h consistency: fix mismatching labels
>   doc txt & -h consistency: add or fix optional "--" syntax
>   doc txt & -h consistency: make output order consistent
>   doc txt & -h consistency: add missing options and labels
>   doc txt & -h consistency: make "rerere" consistent
>   doc txt & -h consistency: make "read-tree" consistent
>   doc txt & -h consistency: make "bundle" consistent
>   doc txt & -h consistency: use "git foo" form, not "git-foo"
>   doc txt & -h consistency: add missing options
>   doc txt & -h consistency: make "stash" consistent
>   doc txt & -h consistency: make "annotate" consistent
>   doc txt & -h consistency: use "[<label>...]" for "zero or more"
>   doc txt & -h consistency: make "diff-tree" consistent
>   doc txt & -h consistency: make "commit" consistent
>   reflog doc: list real subcommands up-front
>   worktree: define subcommand -h in terms of command -h
>   doc txt & -h consistency: make "worktree" consistent
>   tests: start asserting that *.txt SYNOPSIS matches -h output
>