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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 11, 2021, 01:53 UTC
Message-ID: <878s03c1of.fsf@evledraar.gmail.com>
In-Reply-To: <CAPig+cQdAuLkZ0pDK6XOfm_WXCJAOm8Tr19oK14n-Tf7DcfW=w@mail.gmail.com>

On Fri, Sep 10 2021, Eric Sunshine wrote:

Show 21 quoted lines

> On Fri, Sep 10, 2021 at 2:33 PM Junio C Hamano <gitster@pobox.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?
>>
>> I do not know about "have we made" part of the question, but an
>> input from me to come to an answer to the question is that, while I
>> can see why it may be desirable in some cases, I do not view it as
>> compelling enough to risk any unforeseen breakage to other peoples'
>> workflow.  My opinion is based on an assumption that it is desirable
>> because it would squelch "here is a trailing whitespace" noise in an
>> editor and/or a pager that is appropriately configured and allow the
>> user to spot whitespace breakages in the payload more easily and for
>> no other reason.  If there are other reasons that make this change
>> desirable, they might influence my opinion.
>
> Thank you for the response. I didn't have any other reason beyond
> squelching "here is trailing whitespace" noise when submitting the
> series. Thus, I can't provide any other reasons to promote the change
> as desirable.

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/'
    commit c990a4c11ddZ
    Author: Eric Sunshine <sunshine@sunshineco.com>Z
    Date:   Mon Jul 6 13:30:45 2015 -0400Z
    Z
        checkout: fix bug with --to and relative HEADZ
        Z
        Given "git checkout --to <path> HEAD~1", the new worktree's HEAD shouldZ
        begin life at the current branch's HEAD~1, however, it actually ends upZ
        at HEAD~2. This happens because:Z
        Z
            1. git-checkout resolves HEAD~1Z
        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.

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?

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

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.