Re: [PATCH] builtin/config.c: don't print a newline with --color
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Mar 5, 2019, 15:59 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1903051658200.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20190303172428.GE23811@sigill.intra.peff.net>
Hi Peff,
On Sun, 3 Mar 2019, Jeff King wrote:
Show 17 quoted lines
> 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" ;-)
Show 13 quoted lines
> 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