On Wed, Apr 17, 2013 at 7:33 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> @@ -1005,7 +1006,15 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>> /* these are independent of the commit */
>> switch (placeholder[0]) {
>> case 'C':
>> - return parse_color(sb, placeholder, c);
>> + if (!prefixcmp(placeholder + 1, "(auto)")) {
>> + c->auto_color_next = 1;
>> + return 7;
>> + } else {
>> + int ret = parse_color(sb, placeholder, c);
>> + if (ret)
>> + c->auto_color_next = 0;
>> + return ret;
>> + }
>
> This is to handle a corrupt input, e.g. "%C(auto)%Cbleu%H" where
> (perhaps deprecated) "%Cblue" is misspelled, and parse_color()
> returns 0 without consuming any byte.
>
> Does it make sense not to turn auto off in such a case?
We don't have any mechanism to report invalid %C. Instead we let them
through as literals. If they are literals, they should not have any
side effects. So I think it makes sense not to turn off things.
> Otherwise the above would become
>
> if (!prefixcmp(placeholder + 1, "(auto)")) {
> c->auto_color_next = 1;
> return 7; /* consumed 7 bytes, "C(auto)" */
> }
> c->auto_color_next = 0;
> return parse_color(sb, placeholder, c);
>
> which may be simpler. When we see %C, previous %C(auto) is
> cancelled.
If we do this, maybe we could show invalid %C with blinking. Quite
catchy and might make the user wonder why. Of course it won't work
without coloring. But who would add %C in that case.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html