Re: [PATCH] builtin/config.c: don't print a newline with --color
- Date: Sun, 3 Mar 2019 12:24:29 -0500
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] builtin/config.c: don't print a newline with --color
On Sat, Mar 02, 2019 at 09:25:28PM +0100, Johannes Schindelin wrote:
> > This bug has survived because there was never a test that would have
> > caught it. The old test used 'test_decode_color', which checks that its
> > input begins with a color, but stops parsing once it has parsed a single
> > color successfully. In this case, it was ignoring the trailing '\n'.
> Isn't the reason rather that our test compared the output to an expected
> text *with a newline*? IOW:
> This should do the right thing if you write
> printf "<RED>" >expect
No, there are actually two problems that cancel each other out. Because
test_decode_color() is implemented in awk, it doesn't see if there's
actually a newline or not in each record. So it _always_ adds a newline
after printing out the line (and I don't think Taylor's explanation
above is quite right; it does have a loop to handle multiple colors).
So regardless of whether git-config is sending the newline or not, the
"actual" file will always have a newline in it.
And then to match that, we used "echo" which of course has a newline,
and matches the test_decode_color output.
So you're right that we need to switch to printf. But doing so without
dropping test_decode_color() would mean the test would always fail. We
have to printf the raw bytes, which is what the new test does.