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

["Rubén Justo" <rjusto@gmail.com>]

On 8/10/22 19:46, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>>> 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.
> 

Fair point.  Thank you for being cautious.