Re: [PATCH 3/3] color: add support for 12-bit RGB colors

[Junio C Hamano <gitster@pobox.com>]

"Beat Bolli" <bb@drbeat.li> writes:

> +static int get_hex_color(const char **inp, int width, unsigned char *out)
>  {
> +	const char *in = *inp;
>  	unsigned int val;
> -	val = (hexval(in[0]) << 4) | hexval(in[1]);
> +
> +	assert(width == 1 || width == 2);
> +	val = (hexval(in[0]) << 4) | hexval(in[width - 1]);

So this makes #111111 out of #111 and #ffffff out of #fff.  Nice.

> diff --git a/color.h b/color.h
> index bb28343be210..7ed259a35bb4 100644
> --- a/color.h
> +++ b/color.h
> @@ -112,7 +112,8 @@ int want_color_fd(int fd, int var);
>   * Translate a Git color from 'value' into a string that the terminal can
>   * interpret and store it into 'dst'. The Git color values are of the form
>   * "foreground [background] [attr]" where fore- and background can be a color
> - * name ("red"), a RGB code (#0xFF0000) or a 256-color-mode from the terminal.
> + * name ("red"), a RGB code (#FF0000 or #F00) or a 256-color-mode from the
> + * terminal.
>   */

Good.  Hopefully we do not have such extra 0x in our end-user facing
documentation?

> diff --git a/t/t4026-color.sh b/t/t4026-color.sh
> index 0c39bd74a613..9a6f8a4bc5bf 100755
> --- a/t/t4026-color.sh
> +++ b/t/t4026-color.sh
> @@ -96,8 +96,8 @@ test_expect_success '256 colors' '
>  	color "254 bold 255" "[1;38;5;254;48;5;255m"
>  '
>  
> -test_expect_success '24-bit colors' '
> -	color "#ff00ff black" "[38;2;255;0;255;40m"
> +test_expect_success 'RGB colors' '
> +	color "#ff00ff #0f0" "[38;2;255;0;255;48;2;0;255;0m"
>  '
>  
>  test_expect_success '"default" foreground' '
> @@ -146,7 +146,10 @@ test_expect_success 'non-hex character in RGB color' '
>  	invalid_color "#12x456" &&
>  	invalid_color "#123x56" &&
>  	invalid_color "#1234x6" &&
> -	invalid_color "#12345x"
> +	invalid_color "#12345x" &&
> +	invalid_color "#x23" &&
> +	invalid_color "#1x3" &&
> +	invalid_color "#12x"
>  '

OK, looking good.

Thanks.