Re: [PATCH 3/3] notes: don't indent empty lines

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Sep 10, 2021 at 9:59 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> >> Eric Sunshine <sunshine@sunshineco.com> writes:
> >> > Have we made a decision about whether this patch series -- which
> >> > avoids indenting blank notes lines -- is desirable? Or are we worried
> >> > about backward-compatibility?
>
> This change per-se seems nice, but even having reviewed it to the point
> of rewriting parts of it, I didn't really look into what the whole
> workflow you were trying to address is.
>
> So e.g. just to pick a random commit of your for show:
>     $ git show c990a4c11dd | sed 's/$/Z/'
> Here we end up also adding the whitespace indenting to the empty lines,
> whereas if we were trying to feed this to an editor we'd place those
> later Z's at the start of our line.

I'm not sure what you mean by "feed this to an editor". Do you mean
sending the output of `git show` to an editor? I'm guessing that's not
what you mean, and that you instead are talking about editing the
commit message in an editor (say, via the "reword" option of `git
rebase --interactive`).

> Are notes different? Or are they just similarly indented? For commits we
> don't insert that leading whitespace in the commit object, do notes get
> that part wrong too?

Notes don't store the indented blank lines; it's only at output time,
such as with `git format-patch --notes` that the blank lines get
indented along with the rest of the note text (just as is happening in
your `git show` example in which the entire commit message is being
indented, including the blank lines).

> It might be showing, but I've only used notes a few times, my main use
> of them is Junio's amlog.

I also have only used notes a few times.

> So even for someone experienced in git, I think some show & tell of
> step-by-step showing in the commit message how we end up with X before,
> and have Y with this change would help a lot.

This all came about due to two unrelated circumstances: (1) a few
months ago, I configured Emacs to highlight trailing whitespace, and
(2) I decided to use `notes` to add commentary to a commit since,
although I normally just write the commentary directly in the patch
itself after running `git format-patch`, in this case, it likely will
be weeks or months before I finish the series, and was worried that
I'd forget the intended commentary by that time, thus recorded it as a
note. Since I've almost never used notes, I ran `git format-patch
--notes` as a test and was surprised to see the trailing whitespace on
the "blank" lines when viewing the patch in the editor.

This submission started as a single patch which just "fixed" the bug
and added a test. Only after that was complete (but before I submitted
the patch), did I discover that other tests in the suite were failing
since the "fix" also changed git-log's default output format which
includes notes (indented). Since I so rarely use notes, I had either
forgotten that git-log showed notes or didn't know in the first place.
The submission grew to multiple patches due to fixing those
newly-failing tests.

Anyhow, since then, I've discovered that `git format-patch
--range-diff` also indents blank lines. And you've now shown that `git
show` does, as well, so the behavior which triggered this "fix" turns
out to be somewhat normal in this project, rather than a one-off "bug"
in need of a fix.