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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 11, 2021, 09:15 UTC
Message-ID: <CAPig+cQ+qVNBJqHmQgk6D1fbYHHJpAxhfwyBOgevi9Hvs6JYkw@mail.gmail.com>
In-Reply-To: <878s03c1of.fsf@evledraar.gmail.com>

On Fri, Sep 10, 2021 at 9:59 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 14 quoted lines

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