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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 27, 2022, 14:20 UTC
Message-ID: <xmqqo7xatz5o.fsf@gitster.g>
In-Reply-To: <Yt8sQ6kPoUUQFjle@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 23 quoted lines

> On Fri, Jul 22, 2022 at 10:35:43PM -0700, Junio C Hamano wrote:
>> Taylor Blau <me@ttaylorr.com> writes:
>>
>> > @@ -14,7 +14,7 @@ SYNOPSIS
>> >  'git cat-file' (-t | -s) [--allow-unknown-type] <object>
>> >  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]
>> >  	     [--buffer] [--follow-symlinks] [--unordered]
>> > -	     [--textconv | --filters]
>> > +	     [--textconv | --filters] [-z]
>>
>> Is "-z" useful with any other option, or is it useful only in
>> combination with one of the three --batch-*?  The above suggests the
>> former.
>
> It only makes sense with `--batch`-related options. But doesn't the
> above suggest the latter, not the former? That synopsis line begins
> with:
>
>     'git cat-file' (--batch | --batch-check | --batch-command) ...
>
> which made me think that this was the invocation for batch-related
> options, and only listed options that made sense with a `--batch` mode
> of one kind or another.

Ah, yes you are absolutely right.  I misread the synopsis.

Show 5 quoted lines

> I think the helper should probably be something more like:
>
>     echo_with_nul () {
>         echo "$@" | tr '\n' '\0'
>     }

I think you meant

	printf "%s\n" "$@" | tr ..

but then I wonder if

	printf "%s\000" "$@"

would work better?

Show 9 quoted lines

>> > +	printf "HEAD:newline${LF}embedded" >in &&
>> > +	git cat-file --batch-check -z <in >actual &&
>>
>> As I already said, I suspect that new users who know how our path
>> quoting works would expect c-quoted path would work just fine
>> without using "-z".  It is not a reason to refuse "-z" to exist,
>> though.
>
> Yeah. I think we can do both, if there is a need.

Even though we know it is needed already, it is unfortunately way too late now X-<, because existing scripts know that paths are not taken as c-quoted, even though people would naturally expect that paths are, and satisfying the latter expectation now means breaking existing scripts.

In any case, as long as "-z" is designed right from the day one, it would be fine ;-)

Thanks.