From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Thu, 17 Jun 2021 21:41:44 GMT
Subject: RE: [PATCH v2 3/4] show-branch: don't <COLOR></RESET> for space characters
Message-ID: <60cbc198d0552_1299208e8@natae.notmuch>
In-Reply-To: <patch-3.4-937e728f7f-20210617T105245Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:
> Change the colored output introduced in ab07ba2a24 (show-branch: color
> the commit status signs, 2009-04-22) to not color and reset each
> individual space character we use for padding. The intent is to color
> just the "!", "+" etc. characters.

Obviously a fix, but perhaps show the current behavior:

<RED>+<RESET><GREEN> <RESET><YELLOW> <RESET><BLUE> <RESET><MAGENTA> <RESET><CYAN> <RESET><BOLD;RED> <RESET><BOLD;GREEN> <RESET><BOLD;YELLOW> <RESET><BOLD;BLUE> <RESET> [branch1] branch1

Versus:

<RED>+<RESET>          [branch1] branch1

> In theory this breaks things for anyone who's relying on the exact
> colored output of show-branch, in practice I'd think anyone parsing it
> isn't actively turning on the colored output.

Please let's limit our worries to real users. Few people use
`git show-branch`, even less would rely on parsing its bogus output.

> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
>  					mark = '*';
>  				else
>  					mark = '+';
> -				printf("%s%c%s",
> -				       get_color_code(i),
> -				       mark, get_color_reset_code());
> +				if (mark == ' ')
> +					putchar(mark);
> +				else
> +					printf("%s%c%s",
> +					       get_color_code(i),
> +					       mark, get_color_reset_code());

Very simple and obvious improvement.

Cheers.

-- 
Felipe Contreras