On Thu, Jul 13, 2017 at 7:58 AM, Jeff King <peff@xxxxxxxx> wrote:
> I really only need t6300 and t6006 converted to build on for the rest of
> the series. But t4207 was easy to do. t4026 still uses raw codes, but
> converting it would be a pretty big job, so I punted.
>
I think it is good to have raw codes in at least one place to test
that raw codes work, but then again it could be specific test calling
out that this is tested.
> @@ -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.
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).
> @@ -61,8 +61,9 @@ test_format () {
> # Feed to --format to provide predictable colored sequences.
> AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'
> has_color () {
> - printf '\033[31mfoo\033[m\n' >expect &&
> - test_cmp expect "$1"
> + test_decode_color <"$1" >decoded &&
> + echo "<RED>foo<RESET>" >expect &&
> + test_cmp expect decoded
> }
Thanks for removing hard coded colors :)
