On Thu, Apr 05, 2018 at 06:52:29PM -0400, Eric Sunshine wrote:
> On Thu, Apr 5, 2018 at 6:36 PM, Jeff King <peff@xxxxxxxx> wrote:
> > On Wed, Apr 04, 2018 at 07:59:17PM -0700, Taylor Blau wrote:
> >> + if (type == TYPE_COLOR) {
> >> + char v[COLOR_MAXLEN];
> >> + if (!git_config_color(v, key, value))
> >> + /*
> >> + * The contents of `v` now contain an ANSI escape
> >> + * sequence, not suitable for including within a
> >> + * configuration file. Treat the above as a
> >> + * "sanity-check", and return the given value, which we
> >> + * know is representable as valid color code.
> >> + */
> >> + return xstrdup(value);
> >> + die("cannot parse color '%s'", value);
> >> + }
> >
> > And this returns the original. Good.
>
> It's entirely subjective, but I find this easier to read and
> understand when written like this:
>
> char v[COLOR_MAXLEN];
> if (git_config_color(v, key, value))
> die("cannot parse color '%s'", value);
>
> /*
> * The contents of `v` now contain an ANSI escape
> * sequence, not suitable for including within a
> * configuration file. Treat the above as a
> * "sanity-check", and return the given value, which we
> * know is representable as valid color code.
> */
> return xstrdup(value);
It may be subjective, but I happen to agree with you.
-Peff
