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