Re: [PATCH 2/2] builtin/cat-file.c: support NUL-delimited input with `-z`

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 23, 2022, 17:45 UTC
Message-ID: <xmqqmtczafku.fsf@gitster.g>
In-Reply-To: <220723.86leskqtej.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 14 quoted lines

> This doesn't apply to the rest of cat-file directly, but the origin of
> the recent --batch-command mode cdoe was to lift the same-ish code from
> builtin/update-ref.c, whose \n or \0 mode does exactly that:
>
> 	while (!strbuf_getwholeline(&input, stdin, line_termination)) {
>
> I.e. it doesn't support \r\n, just \n or \0.
>
> Isn't that fine? I may be missing something, but why isn't it OK even on
> platforms that use \r\n for their normal *.txt endings to only use \n or
> \0 for this bit of protocol?
>
> So maybe we should just do something like that instead? I.e. declare
> that a mistake.

Good point.

Show 6 quoted lines

> So isn't this just an accident in of us having used the strbuf_getline()
> function to mean "\n", but actually it also does "\r\n".
>
> Which is a really unfortunately named function b.t.w., since it sneaks
> this bit of Windows portability into places that may not want it in the
> first place.

getline() is to read a single "logical" line, so it is fine for it to strip CRLF on platforms with CRLF, and to leave CR at the end of line on a LF platform. If the "protocol" is defined to use LF on any platform (and allow a payload that ends with CR to be passed), you can argue that it is a wrong helper to call.

But does that result in a sensible behaviour? I am not sure. Some editors that are popular on LF platforms can produce CRLF files when the user asks (either on purpose or by mistake), and when not using the "-z" mode of input, I suspect that most users would expect CR at the end of the "line" (terminated with LF on their platform of choice) would be thrown away even on their LF platform, simply because it is unlikely that the kind of input they are preparing can usefully and legitimately end with CR, as their colleagues on CRLF platforms may also have to produce similar input. IOW, the presence of CRLF platforms makes text lines that end with CR much less useful everywhere.

And from that point of view, "getline() or getwholeline(NUL)" may be a pattern that is practically more useful than the old "use always getwholeline(NUL or LF)" convention that I invented more than 10 years ago.