Re: [GSoC RFC PATCH v2 5/7] repo-info: add the field references.format

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I've concentrated my comments on the tests as others have commented on
> the code itself. In general test bodies should be wrapped in single
> quotes rather than double quotes and one should prefer test_cmp() over
> test_line_count().

>> +                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?

>> +                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 &&

> Best Wishes
>
> Phillip

Thanks.