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.