Re: [PATCH v3 10/13] pretty: add %C(auto) for auto-coloring
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 16, 2013, 21:33 UTC
- Message-ID
- <7vd2tujh3o.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1366100702-31745-11-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 24 quoted lines
> This is not simply convenient over %C(auto,xxx). Some placeholders > (actually only one, %d) do multi coloring and we can't emit a multiple > colors with %C(auto,xxx). > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > Documentation/pretty-formats.txt | 3 ++- > pretty.c | 17 +++++++++++++++-- > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt > index 6bde67e..bad627a 100644 > --- a/Documentation/pretty-formats.txt > +++ b/Documentation/pretty-formats.txt > @@ -156,7 +156,8 @@ The placeholders are: > adding `auto,` at the beginning will emit color only when colors are > enabled for log output (by `color.diff`, `color.ui`, or `--color`, and > respecting the `auto` settings of the former if we are going to a > - terminal) > + terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring > + on the next placeholders until the color is switched again. > - '%m': left, right or boundary mark > - '%n': newline > - '%%': a raw '%'
Good addition.
In the previous round, it used to be "%C(auto)" was remembered in this field to color the _next_ one, and the momorized value was toggled off. As the field no longer toggles automatically once it is used, perhaps it should simply be called "auto_color" without "next", though?
I can locally tweak that before queuing if you think it is better.
Show 26 quoted lines
> diff --git a/pretty.c b/pretty.c
> index e0413e3..f385176 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -778,6 +778,7 @@ struct format_commit_context {
> char *message;
> char *commit_encoding;
> size_t width, indent1, indent2;
> + int auto_color_next;
>
> /* These offsets are relative to the start of the commit message. */
> struct chunk author;
> @@ -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? 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.
Show 11 quoted lines
> case 'n': /* newline */
> strbuf_addch(sb, '\n');
> return 1;
> @@ -1051,13 +1060,17 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>
> switch (placeholder[0]) {
> case 'H': /* commit hash */
> + strbuf_addstr(sb, diff_get_color(c->auto_color_next, DIFF_COMMIT));
> strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
> + strbuf_addstr(sb, diff_get_color(c->auto_color_next, DIFF_RESET));
> return 1;