git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Jeff KingNext: Taylor Blau
Message 5 of 18 in “builtin/config.c: don't print a newline with --color”
  1. builtin/config.c: don't print a newline with --colorTaylor Blau, Mar 2, 2019
  2. Eric SunshineMar 2, 2019
  3. Johannes SchindelinMar 2, 2019
  4. Jeff KingMar 3, 2019
  5. Johannes SchindelinMar 5, 2019
  6. Taylor BlauMar 6, 2019
  7. Junio C HamanoMar 3, 2019
  8. Jeff KingMar 3, 2019
  9. Junio C HamanoMar 4, 2019
  10. Junio C HamanoMar 4, 2019
  11. Jeff KingMar 5, 2019
  12. Junio C HamanoMar 5, 2019
  13. Taylor BlauMar 6, 2019
  14. Junio C HamanoMar 7, 2019
  15. Jeff KingMar 7, 2019
  16. Documentation/config: note trailing newline with --type=colorTaylor Blau, Mar 6, 2019
  17. Jeff KingMar 7, 2019
  18. Junio C HamanoMar 7, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.