* [PATCH 0/3] color: add support for 12-bit RGB colors
@ 2024-04-29 16:48 Beat Bolli
2024-04-29 16:48 ` [PATCH 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Beat Bolli @ 2024-04-29 16:48 UTC (permalink / raw
To: git; +Cc: Jeff King, Beat Bolli
* The color parsing code learned to handle 12-bit RGB colors.
The first commit fixes a typo, the second one adds some test coverage
for invalid RGB colors, and the final one extends the RGB color parser
to recognize 12-bit colors, as in #f0f.
Beat Bolli (3):
t/t4026-color: remove an extra double quote character
t/t4026-color: add test coverage for invalid RGB colors
color: add support for 12-bit RGB colors
Documentation/config.txt | 3 ++-
color.c | 21 ++++++++++++++-------
color.h | 3 ++-
t/t4026-color.sh | 18 +++++++++++++++---
4 files changed, 33 insertions(+), 12 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] t/t4026-color: remove an extra double quote character
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
@ 2024-04-29 16:48 ` Beat Bolli
2024-04-30 10:59 ` Jeff King
2024-04-29 16:48 ` [PATCH 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Beat Bolli @ 2024-04-29 16:48 UTC (permalink / raw
To: git; +Cc: Jeff King, Beat Bolli
This is most probably just an editing left-over from cb357221a4 (t4026:
test "normal" color, 2014-11-20) which added this test.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
t/t4026-color.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index cc3f60d468f4..37622451fc23 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -112,7 +112,7 @@ test_expect_success '"default" can be combined with attributes' '
color "default default no-reverse bold" "[1;27;39;49m"
'
-test_expect_success '"normal" yields no color at all"' '
+test_expect_success '"normal" yields no color at all' '
color "normal black" "[40m"
'
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] t/t4026-color: add test coverage for invalid RGB colors
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
2024-04-29 16:48 ` [PATCH 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
@ 2024-04-29 16:48 ` Beat Bolli
2024-04-29 16:48 ` [PATCH 3/3] color: add support for 12-bit " Beat Bolli
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-04-29 16:48 UTC (permalink / raw
To: git; +Cc: Jeff King, Beat Bolli
Make sure that the RGB color parser rejects invalid characters.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
t/t4026-color.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index 37622451fc23..0c39bd74a613 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -140,6 +140,15 @@ test_expect_success 'extra character after attribute' '
invalid_color "dimX"
'
+test_expect_success 'non-hex character in RGB color' '
+ invalid_color "#x23456" &&
+ invalid_color "#1x3456" &&
+ invalid_color "#12x456" &&
+ invalid_color "#123x56" &&
+ invalid_color "#1234x6" &&
+ invalid_color "#12345x"
+'
+
test_expect_success 'unknown color slots are ignored (diff)' '
git config color.diff.nosuchslotwilleverbedefined white &&
git diff --color
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
2024-04-29 16:48 ` [PATCH 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
2024-04-29 16:48 ` [PATCH 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
@ 2024-04-29 16:48 ` Beat Bolli
2024-04-29 17:23 ` Junio C Hamano
2024-04-30 10:57 ` Jeff King
2024-04-29 21:37 ` [PATCH 0/3] " Taylor Blau
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
4 siblings, 2 replies; 20+ messages in thread
From: Beat Bolli @ 2024-04-29 16:48 UTC (permalink / raw
To: git; +Cc: Jeff King, Beat Bolli
RGB color parsing currently supports 24-bit values in the form #RRGGBB.
As in Cascading Style Sheets (CSS [1]), also allow to specify an RGB color
using only three digits with #RGB.
In this shortened form, each of the digits is – again, as in CSS –
duplicated to convert the color to 24 bits, e.g. #f1b specifies the same
color as #ff11bb.
In color.h, remove the '0x' prefix in the example to match the actual
syntax.
[1] https://developer.mozilla.org/en-US/docs/Web/CSS/hex-color
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Documentation/config.txt | 3 ++-
color.c | 21 ++++++++++++++-------
color.h | 3 ++-
t/t4026-color.sh | 9 ++++++---
4 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 70b448b13262..6f649c997c0f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -316,7 +316,8 @@ terminals, this is usually not the same as setting to "white black".
Colors may also be given as numbers between 0 and 255; these use ANSI
256-color mode (but note that not all terminals may support this). If
your terminal supports it, you may also specify 24-bit RGB values as
-hex, like `#ff0ab3`.
+hex, like `#ff0ab3`, or 12-bit RGB values like `#f1b`, which is
+equivalent to the 24-bit color `#ff11bb`.
+
The accepted attributes are `bold`, `dim`, `ul`, `blink`, `reverse`,
`italic`, and `strike` (for crossed-out or "strikethrough" letters).
diff --git a/color.c b/color.c
index f663c06ac4ed..227a5ab2f42e 100644
--- a/color.c
+++ b/color.c
@@ -64,12 +64,16 @@ static int match_word(const char *word, int len, const char *match)
return !strncasecmp(word, match, len) && !match[len];
}
-static int get_hex_color(const char *in, unsigned char *out)
+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]);
if (val & ~0xff)
return -1;
+ *inp += width;
*out = val;
return 0;
}
@@ -135,11 +139,14 @@ static int parse_color(struct color *out, const char *name, int len)
return 0;
}
- /* Try a 24-bit RGB value */
- if (len == 7 && name[0] == '#') {
- if (!get_hex_color(name + 1, &out->red) &&
- !get_hex_color(name + 3, &out->green) &&
- !get_hex_color(name + 5, &out->blue)) {
+ /* Try a 24- or 12-bit RGB value prefixed with '#' */
+ if ((len == 7 || len == 4) && name[0] == '#') {
+ int width_per_color = (len == 7) ? 2 : 1;
+ const char *color = name + 1;
+
+ if (!get_hex_color(&color, width_per_color, &out->red) &&
+ !get_hex_color(&color, width_per_color, &out->green) &&
+ !get_hex_color(&color, width_per_color, &out->blue)) {
out->type = COLOR_RGB;
return 0;
}
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.
*/
int color_parse(const char *value, char *dst);
int color_parse_mem(const char *value, int len, char *dst);
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"
'
test_expect_success 'unknown color slots are ignored (diff)' '
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-29 16:48 ` [PATCH 3/3] color: add support for 12-bit " Beat Bolli
@ 2024-04-29 17:23 ` Junio C Hamano
2024-04-29 17:42 ` Beat Bolli
2024-04-30 10:57 ` Jeff King
1 sibling, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2024-04-29 17:23 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Jeff King, Beat Bolli
"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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-29 17:23 ` Junio C Hamano
@ 2024-04-29 17:42 ` Beat Bolli
0 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-04-29 17:42 UTC (permalink / raw
To: Junio C Hamano, Beat Bolli; +Cc: git, Jeff King
On 29.04.2024 19:23, Junio C Hamano wrote:
> "Beat Bolli" <bb@drbeat.li> writes:
>
>> 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?
No, this was the only '#0x' I found. config.txt is fine as per the first
hunk.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] color: add support for 12-bit RGB colors
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
` (2 preceding siblings ...)
2024-04-29 16:48 ` [PATCH 3/3] color: add support for 12-bit " Beat Bolli
@ 2024-04-29 21:37 ` Taylor Blau
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
4 siblings, 0 replies; 20+ messages in thread
From: Taylor Blau @ 2024-04-29 21:37 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Jeff King, Beat Bolli
On Mon, Apr 29, 2024 at 06:48:46PM +0200, Beat Bolli wrote:
> Documentation/config.txt | 3 ++-
> color.c | 21 ++++++++++++++-------
> color.h | 3 ++-
> t/t4026-color.sh | 18 +++++++++++++++---
> 4 files changed, 33 insertions(+), 12 deletions(-)
Looks very nice. The first two patches are trivially correct, and I took
a close look at 3/3 and couldn't find any errors.
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Thanks,
Taylor
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-29 16:48 ` [PATCH 3/3] color: add support for 12-bit " Beat Bolli
2024-04-29 17:23 ` Junio C Hamano
@ 2024-04-30 10:57 ` Jeff King
2024-04-30 17:31 ` Junio C Hamano
1 sibling, 1 reply; 20+ messages in thread
From: Jeff King @ 2024-04-30 10:57 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Beat Bolli
On Mon, Apr 29, 2024 at 06:48:49PM +0200, Beat Bolli wrote:
> -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"
> '
Heh, I would still think of it as a shorthand for 24-bit color, but I
guess you could argue it is now 12-bit color. :)
(Only observing, I think the new name is fine).
> 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"
> '
This made me wonder what we'd do with "#1", "#12", "#1234", etc. Looking
at the code change, I think we'd continue to reject them. I wonder if it
is worth covering here.
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] t/t4026-color: remove an extra double quote character
2024-04-29 16:48 ` [PATCH 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
@ 2024-04-30 10:59 ` Jeff King
0 siblings, 0 replies; 20+ messages in thread
From: Jeff King @ 2024-04-30 10:59 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Beat Bolli
On Mon, Apr 29, 2024 at 06:48:47PM +0200, Beat Bolli wrote:
> This is most probably just an editing left-over from cb357221a4 (t4026:
> test "normal" color, 2014-11-20) which added this test.
Yeah, I suspect that is correct. Modulo a minor comment I left on the
third patch, the whole series looks good to me.
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-30 10:57 ` Jeff King
@ 2024-04-30 17:31 ` Junio C Hamano
2024-04-30 18:41 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2024-04-30 17:31 UTC (permalink / raw
To: Jeff King; +Cc: Beat Bolli, git, Beat Bolli
Jeff King <peff@peff.net> writes:
> On Mon, Apr 29, 2024 at 06:48:49PM +0200, Beat Bolli wrote:
>
>> -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"
>> '
>
> Heh, I would still think of it as a shorthand for 24-bit color, but I
> guess you could argue it is now 12-bit color. :)
>
> (Only observing, I think the new name is fine).
>
>> 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"
>> '
>
> This made me wonder what we'd do with "#1", "#12", "#1234", etc. Looking
> at the code change, I think we'd continue to reject them. I wonder if it
> is worth covering here.
Worth covering in this test, yes, but I am perfectly OK with leaving
it outside the series as a #leftoverbit clean-up.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-30 17:31 ` Junio C Hamano
@ 2024-04-30 18:41 ` Junio C Hamano
2024-04-30 19:01 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2024-04-30 18:41 UTC (permalink / raw
To: Jeff King; +Cc: Beat Bolli, git, Beat Bolli
Junio C Hamano <gitster@pobox.com> writes:
>>> @@ -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"
>>> '
>>
>> This made me wonder what we'd do with "#1", "#12", "#1234", etc. Looking
>> at the code change, I think we'd continue to reject them. I wonder if it
>> is worth covering here.
>
> Worth covering in this test, yes, but I am perfectly OK with leaving
> it outside the series as a #leftoverbit clean-up.
Ah, I take it back. The preimage was added by [2/3] so it is fair
to say that that step would be the right place to do that from the
get-go.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-30 18:41 ` Junio C Hamano
@ 2024-04-30 19:01 ` Junio C Hamano
2024-05-03 17:47 ` Jeff King
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2024-04-30 19:01 UTC (permalink / raw
To: Jeff King; +Cc: Beat Bolli, git, Beat Bolli
Junio C Hamano <gitster@pobox.com> writes:
> Ah, I take it back. The preimage was added by [2/3] so it is fair
> to say that that step would be the right place to do that from the
> get-go.
Perhaps something like this can be squashed in.
Subject: [PATCH] fixup! t/t4026-color: add test coverage for invalid RGB colors
---
t/t4026-color.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index 9a6f8a4bc5..e60aa588c2 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -140,6 +140,14 @@ test_expect_success 'extra character after attribute' '
invalid_color "dimX"
'
+test_expect_success 'wrong number of letters in RGB color' '
+ invalid_color "#1" &&
+ invalid_color "#23" &&
+ invalid_color "#4567" &&
+ invalid_color "#89abc" &&
+ invalid_color "#def0123"
+'
+
test_expect_success 'non-hex character in RGB color' '
invalid_color "#x23456" &&
invalid_color "#1x3456" &&
--
2.45.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 0/3] color: add support for 12-bit RGB colors
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
` (3 preceding siblings ...)
2024-04-29 21:37 ` [PATCH 0/3] " Taylor Blau
@ 2024-05-02 11:03 ` Beat Bolli
2024-05-02 11:03 ` [PATCH v2 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
` (4 more replies)
4 siblings, 5 replies; 20+ messages in thread
From: Beat Bolli @ 2024-05-02 11:03 UTC (permalink / raw
To: git; +Cc: Jeff King, Junio C Hamano, Beat Bolli
* The color parsing code learned to handle 12-bit RGB colors.
The first commit fixes a typo, the second one adds some test coverage
for invalid RGB colors, and the final one extends the RGB color parser
to recognize 12-bit colors, as in #f0f.
---
Changes against v1:
- add test coverage for invalid RGB color lengths
1: 25da18f71e2c = 1: 25da18f71e2c t/t4026-color: remove an extra double quote character
2: fb9a6ed05279 ! 2: 352fa4c91aa0 t/t4026-color: add test coverage for invalid RGB colors
@@ Metadata
## Commit message ##
t/t4026-color: add test coverage for invalid RGB colors
- Make sure that the RGB color parser rejects invalid characters.
+ Make sure that the RGB color parser rejects invalid characters and
+ invalid lengths.
## t/t4026-color.sh ##
@@ t/t4026-color.sh: test_expect_success 'extra character after attribute' '
@@ t/t4026-color.sh: test_expect_success 'extra character after attribute' '
+ invalid_color "#1234x6" &&
+ invalid_color "#12345x"
+'
++
++test_expect_success 'wrong number of letters in RGB color' '
++ invalid_color "#1" &&
++ invalid_color "#23" &&
++ invalid_color "#456" &&
++ invalid_color "#789a" &&
++ invalid_color "#bcdef" &&
++ invalid_color "#1234567"
++'
+
test_expect_success 'unknown color slots are ignored (diff)' '
git config color.diff.nosuchslotwilleverbedefined white &&
3: 9d109fadcdb1 ! 3: 9147902f698f color: add support for 12-bit RGB colors
@@ t/t4026-color.sh: test_expect_success 'non-hex character in RGB color' '
+ invalid_color "#12x"
'
- test_expect_success 'unknown color slots are ignored (diff)' '
+ test_expect_success 'wrong number of letters in RGB color' '
+ invalid_color "#1" &&
+ invalid_color "#23" &&
+- invalid_color "#456" &&
+ invalid_color "#789a" &&
+ invalid_color "#bcdef" &&
+ invalid_color "#1234567"
Beat Bolli (3):
t/t4026-color: remove an extra double quote character
t/t4026-color: add test coverage for invalid RGB colors
color: add support for 12-bit RGB colors
Documentation/config.txt | 3 ++-
color.c | 21 ++++++++++++++-------
color.h | 3 ++-
t/t4026-color.sh | 26 +++++++++++++++++++++++---
4 files changed, 41 insertions(+), 12 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] t/t4026-color: remove an extra double quote character
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
@ 2024-05-02 11:03 ` Beat Bolli
2024-05-02 11:03 ` [PATCH v2 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-05-02 11:03 UTC (permalink / raw
To: git; +Cc: Jeff King, Junio C Hamano, Beat Bolli
This is most probably just an editing left-over from cb357221a4 (t4026:
test "normal" color, 2014-11-20) which added this test.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
t/t4026-color.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index cc3f60d468f4..37622451fc23 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -112,7 +112,7 @@ test_expect_success '"default" can be combined with attributes' '
color "default default no-reverse bold" "[1;27;39;49m"
'
-test_expect_success '"normal" yields no color at all"' '
+test_expect_success '"normal" yields no color at all' '
color "normal black" "[40m"
'
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] t/t4026-color: add test coverage for invalid RGB colors
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
2024-05-02 11:03 ` [PATCH v2 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
@ 2024-05-02 11:03 ` Beat Bolli
2024-05-02 11:03 ` [PATCH v2 3/3] color: add support for 12-bit " Beat Bolli
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-05-02 11:03 UTC (permalink / raw
To: git; +Cc: Jeff King, Junio C Hamano, Beat Bolli
Make sure that the RGB color parser rejects invalid characters and
invalid lengths.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
t/t4026-color.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index 37622451fc23..c41138031989 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -140,6 +140,24 @@ test_expect_success 'extra character after attribute' '
invalid_color "dimX"
'
+test_expect_success 'non-hex character in RGB color' '
+ invalid_color "#x23456" &&
+ invalid_color "#1x3456" &&
+ invalid_color "#12x456" &&
+ invalid_color "#123x56" &&
+ invalid_color "#1234x6" &&
+ invalid_color "#12345x"
+'
+
+test_expect_success 'wrong number of letters in RGB color' '
+ invalid_color "#1" &&
+ invalid_color "#23" &&
+ invalid_color "#456" &&
+ invalid_color "#789a" &&
+ invalid_color "#bcdef" &&
+ invalid_color "#1234567"
+'
+
test_expect_success 'unknown color slots are ignored (diff)' '
git config color.diff.nosuchslotwilleverbedefined white &&
git diff --color
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] color: add support for 12-bit RGB colors
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
2024-05-02 11:03 ` [PATCH v2 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
2024-05-02 11:03 ` [PATCH v2 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
@ 2024-05-02 11:03 ` Beat Bolli
2024-05-02 16:29 ` [PATCH v2 0/3] " Junio C Hamano
2024-05-03 17:48 ` Jeff King
4 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-05-02 11:03 UTC (permalink / raw
To: git; +Cc: Jeff King, Junio C Hamano, Beat Bolli
RGB color parsing currently supports 24-bit values in the form #RRGGBB.
As in Cascading Style Sheets (CSS [1]), also allow to specify an RGB color
using only three digits with #RGB.
In this shortened form, each of the digits is – again, as in CSS –
duplicated to convert the color to 24 bits, e.g. #f1b specifies the same
color as #ff11bb.
In color.h, remove the '0x' prefix in the example to match the actual
syntax.
[1] https://developer.mozilla.org/en-US/docs/Web/CSS/hex-color
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Documentation/config.txt | 3 ++-
color.c | 21 ++++++++++++++-------
color.h | 3 ++-
t/t4026-color.sh | 10 ++++++----
4 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 70b448b13262..6f649c997c0f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -316,7 +316,8 @@ terminals, this is usually not the same as setting to "white black".
Colors may also be given as numbers between 0 and 255; these use ANSI
256-color mode (but note that not all terminals may support this). If
your terminal supports it, you may also specify 24-bit RGB values as
-hex, like `#ff0ab3`.
+hex, like `#ff0ab3`, or 12-bit RGB values like `#f1b`, which is
+equivalent to the 24-bit color `#ff11bb`.
+
The accepted attributes are `bold`, `dim`, `ul`, `blink`, `reverse`,
`italic`, and `strike` (for crossed-out or "strikethrough" letters).
diff --git a/color.c b/color.c
index f663c06ac4ed..227a5ab2f42e 100644
--- a/color.c
+++ b/color.c
@@ -64,12 +64,16 @@ static int match_word(const char *word, int len, const char *match)
return !strncasecmp(word, match, len) && !match[len];
}
-static int get_hex_color(const char *in, unsigned char *out)
+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]);
if (val & ~0xff)
return -1;
+ *inp += width;
*out = val;
return 0;
}
@@ -135,11 +139,14 @@ static int parse_color(struct color *out, const char *name, int len)
return 0;
}
- /* Try a 24-bit RGB value */
- if (len == 7 && name[0] == '#') {
- if (!get_hex_color(name + 1, &out->red) &&
- !get_hex_color(name + 3, &out->green) &&
- !get_hex_color(name + 5, &out->blue)) {
+ /* Try a 24- or 12-bit RGB value prefixed with '#' */
+ if ((len == 7 || len == 4) && name[0] == '#') {
+ int width_per_color = (len == 7) ? 2 : 1;
+ const char *color = name + 1;
+
+ if (!get_hex_color(&color, width_per_color, &out->red) &&
+ !get_hex_color(&color, width_per_color, &out->green) &&
+ !get_hex_color(&color, width_per_color, &out->blue)) {
out->type = COLOR_RGB;
return 0;
}
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.
*/
int color_parse(const char *value, char *dst);
int color_parse_mem(const char *value, int len, char *dst);
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index c41138031989..b05f2a9b6075 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,13 +146,15 @@ 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"
'
test_expect_success 'wrong number of letters in RGB color' '
invalid_color "#1" &&
invalid_color "#23" &&
- invalid_color "#456" &&
invalid_color "#789a" &&
invalid_color "#bcdef" &&
invalid_color "#1234567"
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
` (2 preceding siblings ...)
2024-05-02 11:03 ` [PATCH v2 3/3] color: add support for 12-bit " Beat Bolli
@ 2024-05-02 16:29 ` Junio C Hamano
2024-05-03 17:48 ` Jeff King
4 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2024-05-02 16:29 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Jeff King, Beat Bolli
"Beat Bolli" <bb@drbeat.li> writes:
> * The color parsing code learned to handle 12-bit RGB colors.
>
> The first commit fixes a typo, the second one adds some test coverage
> for invalid RGB colors, and the final one extends the RGB color parser
> to recognize 12-bit colors, as in #f0f.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] color: add support for 12-bit RGB colors
2024-04-30 19:01 ` Junio C Hamano
@ 2024-05-03 17:47 ` Jeff King
0 siblings, 0 replies; 20+ messages in thread
From: Jeff King @ 2024-05-03 17:47 UTC (permalink / raw
To: Junio C Hamano; +Cc: Beat Bolli, git, Beat Bolli
On Tue, Apr 30, 2024 at 12:01:54PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Ah, I take it back. The preimage was added by [2/3] so it is fair
> > to say that that step would be the right place to do that from the
> > get-go.
>
> Perhaps something like this can be squashed in.
>
> Subject: [PATCH] fixup! t/t4026-color: add test coverage for invalid RGB colors
Yup, that looks good to me.
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
` (3 preceding siblings ...)
2024-05-02 16:29 ` [PATCH v2 0/3] " Junio C Hamano
@ 2024-05-03 17:48 ` Jeff King
2024-05-03 19:31 ` Beat Bolli
4 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2024-05-03 17:48 UTC (permalink / raw
To: Beat Bolli; +Cc: git, Junio C Hamano, Beat Bolli
On Thu, May 02, 2024 at 01:03:28PM +0200, Beat Bolli wrote:
> * The color parsing code learned to handle 12-bit RGB colors.
>
> The first commit fixes a typo, the second one adds some test coverage
> for invalid RGB colors, and the final one extends the RGB color parser
> to recognize 12-bit colors, as in #f0f.
> ---
>
> Changes against v1:
> - add test coverage for invalid RGB color lengths
Ah, I missed that you had sent a new version when I responded to Junio.
Yeah, this whole thing looks good to me!
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors
2024-05-03 17:48 ` Jeff King
@ 2024-05-03 19:31 ` Beat Bolli
0 siblings, 0 replies; 20+ messages in thread
From: Beat Bolli @ 2024-05-03 19:31 UTC (permalink / raw
To: Jeff King; +Cc: git, Junio C Hamano
On 03.05.2024 19:48, Jeff King wrote:
> On Thu, May 02, 2024 at 01:03:28PM +0200, Beat Bolli wrote:
>
>> * The color parsing code learned to handle 12-bit RGB colors.
>>
>> The first commit fixes a typo, the second one adds some test coverage
>> for invalid RGB colors, and the final one extends the RGB color parser
>> to recognize 12-bit colors, as in #f0f.
>> ---
>>
>> Changes against v1:
>> - add test coverage for invalid RGB color lengths
>
> Ah, I missed that you had sent a new version when I responded to Junio.
> Yeah, this whole thing looks good to me!
All good ;-)
Thanks for reviewing!
Cheers, Beat
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-05-03 19:32 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-29 16:48 [PATCH 0/3] color: add support for 12-bit RGB colors Beat Bolli
2024-04-29 16:48 ` [PATCH 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
2024-04-30 10:59 ` Jeff King
2024-04-29 16:48 ` [PATCH 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
2024-04-29 16:48 ` [PATCH 3/3] color: add support for 12-bit " Beat Bolli
2024-04-29 17:23 ` Junio C Hamano
2024-04-29 17:42 ` Beat Bolli
2024-04-30 10:57 ` Jeff King
2024-04-30 17:31 ` Junio C Hamano
2024-04-30 18:41 ` Junio C Hamano
2024-04-30 19:01 ` Junio C Hamano
2024-05-03 17:47 ` Jeff King
2024-04-29 21:37 ` [PATCH 0/3] " Taylor Blau
2024-05-02 11:03 ` [PATCH v2 " Beat Bolli
2024-05-02 11:03 ` [PATCH v2 1/3] t/t4026-color: remove an extra double quote character Beat Bolli
2024-05-02 11:03 ` [PATCH v2 2/3] t/t4026-color: add test coverage for invalid RGB colors Beat Bolli
2024-05-02 11:03 ` [PATCH v2 3/3] color: add support for 12-bit " Beat Bolli
2024-05-02 16:29 ` [PATCH v2 0/3] " Junio C Hamano
2024-05-03 17:48 ` Jeff King
2024-05-03 19:31 ` Beat Bolli
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).