Re: [PATCH v4] branch: support for shortcuts like @{-1}, completed

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 8, 2022, 17:46 UTC
Message-ID: <xmqq1qrib435.fsf@gitster.g>
In-Reply-To: <CAPig+cRxy5C+CqUOzmhe16j+hssxsygha3huVga8tLJ+imM4Hw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 11 quoted lines

>> Yeah, I thought about that.  What convinced me to use "git stripspace" was
>> that maybe that '\n' tail could be removed sometime from the description
>> setting and this will be fine with that.  I haven't found any reason for
>> that '\n' and it bugs me a little seeing it in the config :-)
>
> That reasoning occurred to me, as well, and I'd have no objection to
> git-stripspace if that's the motivation for using it. I don't feel
> strongly one way or the other, and my previous email was intended
> primarily to point out the lightweight alternatives in case you hadn't
> considered them. Feel free to use git-stripspace if you feel it is the
> more appropriate choice.

I do not think I would agree with the line of reasoning.

It all depends on why we anticipate that the terminating LF may go away someday, but if it is because we may do so by mistake and without a good reason when making some unrelated changes to the implementation of "git branch --edit-desc", we would want to know about it, and such a loose check that would miss it is definitely unwelcome. It is very likely that not just "git merge" but people's external scripts depend on the presence of final LF especially when the description has only one line, so unless we are doing deliberately so, we should prepare to catch such a change.

If it is because we may gain a consensus that the description string (which by the way can well consist of multiple lines) is better without the LF on its final line, and we are "fixing" the code to do so very much on purpose, it would be good to have a test to protect such a change from future unintended breakages. Adding a loose test that won't break across such a change today may be OK, but whoever is making such a change in the future has to make sure there is a test that is not loose to protect the change. And it would very likely to be done by adding a new test, instead of noticing that this loosely written test can be tightened to serve the purpose.

So if we start with a tight test that expects the exact number of LFs at the end, we would be better off in that case, too.