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

Re: [PATCH v5 2/3] color.c: trim leading spaces in color_parse_mem()

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 19, 2017, 18:22 UTC
Message-ID
<xmqq60la3o5e.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170119164157.mvoadhxxwwynedoz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> On Thu, Jan 19, 2017 at 06:41:22PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
>> Normally color_parse_mem() is called from config parser which trims the
>> leading spaces already. The new caller in the next patch won't. Let's be
>> tidy and trim leading spaces too (we already trim trailing spaces before
>> comma).
>
> What comma? I don't think that exists until the next patch. :)
>
> I think just trimming from the front is OK, though, because
> color_parse_mem() trims trailing whitespace after a word. So either you
> have a word and we will trim after it, or you do not (in which case
> this will trim everything and hit the !len case you added).
>
> So maybe a better commit message is just:
>
>   Normally color_parse_mem() is called from config parser which trims
>   the leading spaces already. The new caller in the next patch won't.
>   Let's be tidy and trim leading spaces too (we already trim trailing
>   spaces after a word).
>
> -Peff

Yeah, my reading stuttered at the same place in the original, and your rewrite looks a lot more sensible.

Previous: Jeff KingNext: Nguyễn Thái Ngọc Duy
Message 26 of 30 in “log: support 256 colors with --graph=256colors”
  1. log: support 256 colors with --graph=256colorsNguyễn Thái Ngọc Duy, Dec 20, 2016
  2. Jeff KingDec 20, 2016
  3. Duy NguyenDec 22, 2016
  4. Junio C HamanoDec 22, 2016
  5. Duy NguyenDec 25, 2016
  6. Junio C HamanoDec 20, 2016
  7. Duy NguyenDec 22, 2016
  8. log --graph: customize the graph lines with config log.graphColorsNguyễn Thái Ngọc Duy, Dec 24, 2016
  9. Junio C HamanoDec 25, 2016
  10. log --graph: customize the graph lines with config log.graphColorsNguyễn Thái Ngọc Duy, Jan 8, 2017
  11. Junio C HamanoJan 9, 2017
  12. Jeff KingJan 9, 2017
  13. Duy NguyenJan 9, 2017
  14. Junio C HamanoJan 9, 2017
  15. Jeff KingJan 9, 2017
  16. Duy NguyenJan 9, 2017
  17. log --graph: customize the graph lines with config log.graphColorsNguyễn Thái Ngọc Duy, Jan 9, 2017
  18. Junio C HamanoJan 9, 2017
  19. Duy NguyenJan 12, 2017
  20. 0/3 nd/log-graph-configurable-colorsNguyễn Thái Ngọc Duy, Jan 19, 2017
  21. 1/3 color.c: fix color_parse_mem() with value_len == 0Nguyễn Thái Ngọc Duy, Jan 19, 2017
  22. Jeff KingJan 19, 2017
  23. Jeff KingJan 28, 2017
  24. 2/3 color.c: trim leading spaces in color_parse_mem()Nguyễn Thái Ngọc Duy, Jan 19, 2017
  25. Jeff KingJan 19, 2017
  26. Junio C HamanoJan 19, 2017
  27. 3/3 log --graph: customize the graph lines with config log.graphColorsNguyễn Thái Ngọc Duy, Jan 19, 2017
  28. Jeff KingJan 19, 2017
  29. Junio C HamanoJan 19, 2017
  30. Junio C HamanoJan 19, 2017

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.