Web lists-archives.com

Re: [PATCH] builtin/config.c: don't print a newline with --color




Hi Peff,

On Sun, 3 Mar 2019, Jeff King wrote:

> 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.

Ah, so it is actually another instance of "we really need a precise
test-tool command for this rather than a somewhat incomplete and fragile
script" ;-)

> 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.

Fair enough.

Ciao,
Dscho