Re: [PATCH] docs: correct trailer `key_value_separator` description

From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: Mar 18, 2024, 18:15 UTC
Message-ID: <f6a16989-cbcb-4558-ae3b-350437fda7c2@app.fastmail.com>
In-Reply-To: <xmqqh6h3jzp1.fsf@gitster.g>

On Mon, Mar 18, 2024, at 17:02, Junio C Hamano wrote:

Show 32 quoted lines

> Linus Arver <linusa@google.com> writes:
>
>> WRT line lengths, probably 80-ish columns is the (unwritten?) rule. The
>
> Your patches will be reviewed on the mailing list.  If you keep your
> line length to somewhere around ~70, the line will still fit within
> the 80-ish terminal width after a few rounds of review exchanges,
> with ">> " prefixed.  That reasoning is mostly about the proposed
> commit log messages, but the same would apply to things like
> AsciiDoc sources.
>
> It is true that we do not write it down.  Perhaps something like
> this is in order?
>
> diff --git i/Documentation/SubmittingPatches
> w/Documentation/SubmittingPatches
> index e734a3f0f1..68e9ad71a1 100644
> --- i/Documentation/SubmittingPatches
> +++ w/Documentation/SubmittingPatches
> @@ -280,6 +280,14 @@ or, on an older version of Git without support for
> --pretty=reference:
>  	git show -s --date=short --pretty='format:%h (%s, %ad)' <commit>
>  ....
>
> +[[line-wrap]]
> +
> +Just like we limit the patch subject to 50 chars or so, the lines in
> +the proposed log message should be around 70 chars to make sure that
> +it still can be shown on 80-column terminal without line wrapping
> +after a handful of review exchanges add "> " prefix to them.
> +
> +

There’s also `.editorconfig` which says that it should be 72 characters. My Magit respects it but NeoVim doesn’t seem to. Maybe worth mentioning since you might not need to configure it yourself for this project, depending on your commit message editor.

Show 18 quoted lines

>  [[sign-off]]
>  === Certify your work by adding your `Signed-off-by` trailer
>
>
>> text files aren't really meant for end-user consumption (that's what the
>> manpage and HTML formats are for), so I think it's OK if the line
>> lengths are roughly in the same ballpark (no need to worry too much
>> about exact lengths).
>
> Yes, too.  And it is one way to reduce patch noise and nicer to
> reviewers, when used moderately (i.e. removing a word and making a
> line to occupy only 50 columns when ajacent ones are 70 columns may
> still be better than reflowing.  Leaving only a single word on such
> a line may not be reasonable and tucking the word after or before
> one of these ajacent 70-column lines would work better in such a
> case).
>
> Thanks.

My interpretation of this is

1. Commit messages are flowed/reflowed to 72 columns
2. Code is reflowed to 80 columns (enforced by tools like clang-format)
   • See `.clang-format` and `.editorconfig` (kept in synch.)
3. Source documentation (AsciiDoc) is reflowed to 72 opportunistically;
   not every time (in order to avoid diff noise) but when it feels like it
   makes sense

Maybe SubmittingPatches should mention that last point? If my interpretation is correct.