From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 25 Jun 2025 08:40:22 GMT
Subject: Re: [GSoC RFC PATCH v2 5/7] repo-info: add the field references.format
Message-ID: <e892fe89-045f-4220-8436-2d536f10e1a2@gmail.com>
In-Reply-To: <xmqq8qlh2mc0.fsf@gitster.g>

On 24/06/2025 16:25, Junio C Hamano wrote:
 > >>> +                echo '$expected_value' >expect &&
>>> +                git repo-info '$key' >output &&
>>> +                cat output | parse_json >parsed &&
> 
> Running "cat" on a single file and piping it to anything is an
> anti-pattern.  The fact that you can pipe output into the downstream
> command means that the downstream command is prepared to read from
> its standard input, so
> 
> 	parse_json <output >parsed &&
> 
> should be sufficient, right?

Good catch, I commented on this in a test below this one but missed it here.

>>> +                grep -F 'row[0].$key' parsed | cut -d ' ' -f 2 >value &&
>>> +                cat value | sed 's/^0$/false/' | sed 's/^1$/true/' >actual &&
>>
>> sed accepts filenames so there is no need to use "cat" here. It also
>> accepts multiple expressions so you only need a single command
>>
>>      sed "s/^0\$/false/; s/^1\$/true/" value >actual &&
> 
> And you probably do not even need grep piped into cut either, as sed
> is a powerful enough language.  We can also cheat a bit by taking
> advantage of the fact that the characters used in keys are fairly
> tightly controlled, so perhaps something along this line?
> 
> 	sed -n -e "/row[0].$key/{
> 		s/^[^ ]* //
>                  s/^1\$/true/
> 		s/^0\$/false/
>                  p;
> 	}" parsed >actual &&

Nice, as well as saving a couple of processes this avoids truncating 
values that contain a space.

Thanks

Phillip