Jeff King <peff@xxxxxxxx> writes: >> > @@ -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. > 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.