Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
- From
Jeff King <peff@peff.net>
- Date
- Jan 18, 2009, 17:28 UTC
- Message-ID
- <20090118172802.GA17434@coredump.intra.peff.net>
- In-Reply-To
- <4973628C.8080501@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 06:10:36PM +0100, René Scharfe wrote:
Show 5 quoted lines
> > - right now, it is implemented in terms of color_parse(). > > But it would be more efficient to reverse this and > > implement color_parse in terms of color_parse_mem. > > Thusly?
Yes, except for the bugs you introduced. :)
Show 5 quoted lines
> +void color_parse_mem(const char *value, int len, const char *var, char *dst)
> +{
> const char *ptr = value;
> int attr = -1;
> int fg = -2;What's missing in the context here (because it wasn't changed) is:
> if (!strcasecmp(value, "reset")) {
> strcpy(dst, "\033[m");
> return;
> }which doesn't work, since our string is actually something like "reset)\0" or even "reset)some totally unrelated string". So we would need a "memcasecmp" here.
And then in the error case, we call:
> die("bad color value '%s' for variable '%s'", value, var);which is also bogus.
I don't know if this is really even worth it. The timing difference is pretty minimal:
$ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null real 0m0.673s user 0m0.652s sys 0m0.016s $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null real 0m0.692s user 0m0.660s sys 0m0.032s
That's about 1 microsecond per commit.
-Peff