Re: [PATCH v2 09/12] pretty: add %C(auto) for auto-coloring on the next placeholder

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2013, 18:26 UTC
Message-ID: <7vzjxihztj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1364636112-15065-10-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                         | 15 +++++++++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 66345d1..8734224 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -154,7 +154,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 following placeholder.
>  - '%m': left, right or boundary mark
>  - '%n': newline
>  - '%%': a raw '%'

I like this at the conceptual level.

If you say "%C(auto)%C(red)Text%C(auto)%C(reset)", does it do the right thing when the output is not capable of color?

I am a bit worried if the placement of the "grab c->auto_color to decide if we paint for this round and reset it" is optimial and will stay optimal as we enhance format_commit_one() later. Is there a reason why we do not do that at the beginning of the function, before "these are independent of the commit" comment?

Side note. Should the new field called "auto_color_next" or something?