Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
Show 8 quoted lines
> On Sun, Jan 18, 2009 at 06:10:36PM +0100, René Scharfe wrote:
>
>>> - 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 16 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. if (!strncasecmp(value, "reset", len)) {Show 5 quoted lines
> And then in the error case, we call:
>
>> die("bad color value '%s' for variable '%s'", value, var);
>
> which is also bogus. die("bad color value '%.*s' for variable '%s', len, value, var);Show 13 quoted lines
> 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.
Hmm, not too much overhead, agreed, but it would still be nice to avoid it.
René
René Scharfe <rene.scharfe@lsrfire.ath.cx>