Re: [PATCH 03/15] t: use test_decode_color rather than literal ANSI codes

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 13, 2017, 19:27 UTC
Message-ID: <xmqqeftkw30y.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170713184541.ferc3eif7j4mazpg@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 20 quoted lines

>> > @@ -59,7 +54,8 @@ EOF
>> >  # to this test since it does not contain any decoration, hence --first-parent
>> >  test_expect_success 'Commit Decorations Colored Correctly' '
>> >         git log --first-parent --abbrev=10 --all --decorate --oneline --color=always |
>> > -       sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" >out &&
>> > +       sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" |
>> > +       test_decode_color >out &&
>> 
>> Just some thoughts:
>> 
>> This extension of the pipe-chain is not making it worse as gits exit code
>> is already hidden.
>
> Yes, I noticed the existing pipe-chain. We can fix it while we're here,
> though I think it's not a big deal in practice.
>
>> The sed "s/[0-9a-f]\{10,10\}/COMMIT_ID/" is sort of funny, because
>> I would have expected it to break in the future with e.g. the sha1 to longer
>> hash conversion. But as we specify --abbrev=10, this seems future proof.
>> In an ideal world this would be encapsulated in a function (c.f. t/diff-lib.sh).

Actually, --abbrev=10 does not guarantee that the hex part is always 10 bytes long, so it is not future-proofing, but I expect it would work out fine in practice.

Show 6 quoted lines

> I agree it's a bit gross. Possibly:
>
>   git log --format='%C(auto)%d %s'
>
> would be easier for the test to parse (I'm pretty sure that didn't exist
> back when this test was written).

Yeah, that may make the test easier to read, too.

Thanks.