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
