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

Ævar Arnfjörð Bjarmason wrote:

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

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/b1c44436-92d1-067c-fb0a-be4049f3031b@github.com/ [2] https://lore.kernel.org/git/220908.865yhyl31c.gmgdl@evledraar.gmail.com/