On Wed, Jan 28, 2015 at 12:42:26PM -0800, Junio C Hamano wrote:
> > This is the most minimal fix, but I kind of wonder if it should just be
> > using strbuf_rtrim (or even strbuf_trim) in the first place.
>
> Yeah. Or strbuf_chomp(), which does not exist ;-)
This is not the first time I've seen this chomp/trim distinction come
up. However, the thing that has prevented me from writing strbuf_chomp
is that the trim is almost always a more reasonable choice.
Take this instance. We are opening and reading a whole file. Surely we
need to drop the final newline, which is not interesting. But we are not
just doing that; we are dropping _all_ trailing newlines. So "foo\n\n"
becomes "foo". But "foo\n \n" does not. That doesn't make much sense.
IOW, I would venture to say that chomping like this falls into one of
two categories:
1. You want to clean up any extraneous cruft. Multiple lines, extra
whitespace, etc.
2. You want to read one line, but don't want the trailing newline.
And strbuf_getline already handles case (2).
End mini-rant. :)
> It is tempting to apply this directly to maint and merge up
> immediately, as there is no way this 1-byte change will break things
> (of course that is not necessarily true for random 1-byte changes,
> though).
>
> It sometimes gets really hard to resist that temptation during the
> pre-release freeze period.
That's part of why I did the simplest fix instead of strbuf_rtrim. To
tempt you. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html