{"thread":{"id":"61407","subject":"[PATCH v2 1/3] t/t4026-color: remove an extra double quote character","startedAt":"2024-05-02T11:03:57Z","lastAt":"2024-05-03T19:31:42Z","messageCount":7,"participants":["Beat Bolli","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"493913","messageId":"20240502110331.6347-1-dev+git@drbeat.li","threadId":"61407","inReplyTo":"20240429164849.78509-1-dev+git@drbeat.li","subject":"[PATCH v2 0/3] color: add support for 12-bit RGB colors","fromName":"Beat Bolli","fromEmail":"bb@drbeat.li","sentAt":"2024-05-02T11:03:28Z","receivedAt":"2024-05-02T11:03:57Z","isPatch":true,"sender":{"key":"bb@drbeat.li","avatar":null},"body":" * The color parsing code learned to handle 12-bit RGB colors.\n\nThe first commit fixes a typo, the second one adds some test coverage\nfor invalid RGB colors, and the final one extends the RGB color parser\nto recognize 12-bit colors, as in #f0f.\n---\n\nChanges against v1:\n- add test coverage for invalid RGB color lengths\n\n1: 25da18f71e2c = 1: 25da18f71e2c t/t4026-color: remove an extra double quote character\n2: fb9a6ed05279 ! 2: 352fa4c91aa0 t/t4026-color: add test coverage for invalid RGB colors\n @@ Metadata\n ## Commit message ##\n t/t4026-color: add test coverage for invalid RGB colors\n \n - Make sure that the RGB color parser rejects invalid characters.\n + Make sure that the RGB color parser rejects invalid characters and\n + invalid lengths.\n \n ## t/t4026-color.sh ##\n @@ t/t4026-color.sh: test_expect_success 'extra character after attribute' '\n @@ t/t4026-color.sh: test_expect_success 'extra character after attribute' '\n +\tinvalid_color \"#1234x6\" &&\n +\tinvalid_color \"#12345x\"\n +'\n ++\n ++test_expect_success 'wrong number of letters in RGB color' '\n ++\tinvalid_color \"#1\" &&\n ++\tinvalid_color \"#23\" &&\n ++\tinvalid_color \"#456\" &&\n ++\tinvalid_color \"#789a\" &&\n ++\tinvalid_color \"#bcdef\" &&\n ++\tinvalid_color \"#1234567\"\n ++'\n +\n test_expect_success 'unknown color slots are ignored (diff)' '\n \tgit config color.diff.nosuchslotwilleverbedefined white &&\n3: 9d109fadcdb1 ! 3: 9147902f698f color: add support for 12-bit RGB colors\n @@ t/t4026-color.sh: test_expect_success 'non-hex character in RGB color' '\n +\tinvalid_color \"#12x\"\n '\n \n - test_expect_success 'unknown color slots are ignored (diff)' '\n + test_expect_success 'wrong number of letters in RGB color' '\n + \tinvalid_color \"#1\" &&\n + \tinvalid_color \"#23\" &&\n +-\tinvalid_color \"#456\" &&\n + \tinvalid_color \"#789a\" &&\n + \tinvalid_color \"#bcdef\" &&\n + \tinvalid_color \"#1234567\"\n\n\nBeat Bolli (3):\n t/t4026-color: remove an extra double quote character\n t/t4026-color: add test coverage for invalid RGB colors\n color: add support for 12-bit RGB colors\n\n Documentation/config.txt | 3 ++-\n color.c | 21 ++++++++++++++-------\n color.h | 3 ++-\n t/t4026-color.sh | 26 +++++++++++++++++++++++---\n 4 files changed, 41 insertions(+), 12 deletions(-)\n\n-- \n2.44.0\n\n"},{"id":"493910","messageId":"20240502110331.6347-2-dev+git@drbeat.li","threadId":"61407","inReplyTo":"20240502110331.6347-1-dev+git@drbeat.li","subject":"[PATCH v2 1/3] t/t4026-color: remove an extra double quote character","fromName":"Beat Bolli","fromEmail":"bb@drbeat.li","sentAt":"2024-05-02T11:03:29Z","receivedAt":"2024-05-02T11:03:58Z","isPatch":true,"sender":{"key":"bb@drbeat.li","avatar":null},"body":"This is most probably just an editing left-over from cb357221a4 (t4026:\ntest \"normal\" color, 2014-11-20) which added this test.\n\nSigned-off-by: Beat Bolli \n---\n t/t4026-color.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex cc3f60d468f4..37622451fc23 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -112,7 +112,7 @@ test_expect_success '\"default\" can be combined with attributes' '\n \tcolor \"default default no-reverse bold\" \"[1;27;39;49m\"\n '\n \n-test_expect_success '\"normal\" yields no color at all\"' '\n+test_expect_success '\"normal\" yields no color at all' '\n \tcolor \"normal black\" \"[40m\"\n '\n \n-- \n2.44.0\n\n"},{"id":"493911","messageId":"20240502110331.6347-3-dev+git@drbeat.li","threadId":"61407","inReplyTo":"20240502110331.6347-1-dev+git@drbeat.li","subject":"[PATCH v2 2/3] t/t4026-color: add test coverage for invalid RGB colors","fromName":"Beat Bolli","fromEmail":"bb@drbeat.li","sentAt":"2024-05-02T11:03:30Z","receivedAt":"2024-05-02T11:03:59Z","isPatch":true,"sender":{"key":"bb@drbeat.li","avatar":null},"body":"Make sure that the RGB color parser rejects invalid characters and\ninvalid lengths.\n\nSigned-off-by: Beat Bolli \n---\n t/t4026-color.sh | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex 37622451fc23..c41138031989 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -140,6 +140,24 @@ test_expect_success 'extra character after attribute' '\n \tinvalid_color \"dimX\"\n '\n \n+test_expect_success 'non-hex character in RGB color' '\n+\tinvalid_color \"#x23456\" &&\n+\tinvalid_color \"#1x3456\" &&\n+\tinvalid_color \"#12x456\" &&\n+\tinvalid_color \"#123x56\" &&\n+\tinvalid_color \"#1234x6\" &&\n+\tinvalid_color \"#12345x\"\n+'\n+\n+test_expect_success 'wrong number of letters in RGB color' '\n+\tinvalid_color \"#1\" &&\n+\tinvalid_color \"#23\" &&\n+\tinvalid_color \"#456\" &&\n+\tinvalid_color \"#789a\" &&\n+\tinvalid_color \"#bcdef\" &&\n+\tinvalid_color \"#1234567\"\n+'\n+\n test_expect_success 'unknown color slots are ignored (diff)' '\n \tgit config color.diff.nosuchslotwilleverbedefined white &&\n \tgit diff --color\n-- \n2.44.0\n\n"},{"id":"493912","messageId":"20240502110331.6347-4-dev+git@drbeat.li","threadId":"61407","inReplyTo":"20240502110331.6347-1-dev+git@drbeat.li","subject":"[PATCH v2 3/3] color: add support for 12-bit RGB colors","fromName":"Beat Bolli","fromEmail":"bb@drbeat.li","sentAt":"2024-05-02T11:03:31Z","receivedAt":"2024-05-02T11:04:00Z","isPatch":true,"sender":{"key":"bb@drbeat.li","avatar":null},"body":"RGB color parsing currently supports 24-bit values in the form #RRGGBB.\n\nAs in Cascading Style Sheets (CSS [1]), also allow to specify an RGB color\nusing only three digits with #RGB.\n\nIn this shortened form, each of the digits is – again, as in CSS –\nduplicated to convert the color to 24 bits, e.g. #f1b specifies the same\ncolor as #ff11bb.\n\nIn color.h, remove the '0x' prefix in the example to match the actual\nsyntax.\n\n[1] https://developer.mozilla.org/en-US/docs/Web/CSS/hex-color\n\nSigned-off-by: Beat Bolli \n---\n Documentation/config.txt | 3 ++-\n color.c | 21 ++++++++++++++-------\n color.h | 3 ++-\n t/t4026-color.sh | 10 ++++++----\n 4 files changed, 24 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 70b448b13262..6f649c997c0f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -316,7 +316,8 @@ terminals, this is usually not the same as setting to \"white black\".\n Colors may also be given as numbers between 0 and 255; these use ANSI\n 256-color mode (but note that not all terminals may support this). If\n your terminal supports it, you may also specify 24-bit RGB values as\n-hex, like `#ff0ab3`.\n+hex, like `#ff0ab3`, or 12-bit RGB values like `#f1b`, which is\n+equivalent to the 24-bit color `#ff11bb`.\n +\n The accepted attributes are `bold`, `dim`, `ul`, `blink`, `reverse`,\n `italic`, and `strike` (for crossed-out or \"strikethrough\" letters).\ndiff --git a/color.c b/color.c\nindex f663c06ac4ed..227a5ab2f42e 100644\n--- a/color.c\n+++ b/color.c\n@@ -64,12 +64,16 @@ static int match_word(const char *word, int len, const char *match)\n \treturn !strncasecmp(word, match, len) && !match[len];\n }\n \n-static int get_hex_color(const char *in, unsigned char *out)\n+static int get_hex_color(const char **inp, int width, unsigned char *out)\n {\n+\tconst char *in = *inp;\n \tunsigned int val;\n-\tval = (hexval(in[0]) << 4) | hexval(in[1]);\n+\n+\tassert(width == 1 || width == 2);\n+\tval = (hexval(in[0]) << 4) | hexval(in[width - 1]);\n \tif (val & ~0xff)\n \t\treturn -1;\n+\t*inp += width;\n \t*out = val;\n \treturn 0;\n }\n@@ -135,11 +139,14 @@ static int parse_color(struct color *out, const char *name, int len)\n \t\treturn 0;\n \t}\n \n-\t/* Try a 24-bit RGB value */\n-\tif (len == 7 && name[0] == '#') {\n-\t\tif (!get_hex_color(name + 1, &out->red) &&\n-\t\t !get_hex_color(name + 3, &out->green) &&\n-\t\t !get_hex_color(name + 5, &out->blue)) {\n+\t/* Try a 24- or 12-bit RGB value prefixed with '#' */\n+\tif ((len == 7 || len == 4) && name[0] == '#') {\n+\t\tint width_per_color = (len == 7) ? 2 : 1;\n+\t\tconst char *color = name + 1;\n+\n+\t\tif (!get_hex_color(&color, width_per_color, &out->red) &&\n+\t\t !get_hex_color(&color, width_per_color, &out->green) &&\n+\t\t !get_hex_color(&color, width_per_color, &out->blue)) {\n \t\t\tout->type = COLOR_RGB;\n \t\t\treturn 0;\n \t\t}\ndiff --git a/color.h b/color.h\nindex bb28343be210..7ed259a35bb4 100644\n--- a/color.h\n+++ b/color.h\n@@ -112,7 +112,8 @@ int want_color_fd(int fd, int var);\n * Translate a Git color from 'value' into a string that the terminal can\n * interpret and store it into 'dst'. The Git color values are of the form\n * \"foreground [background] [attr]\" where fore- and background can be a color\n- * name (\"red\"), a RGB code (#0xFF0000) or a 256-color-mode from the terminal.\n+ * name (\"red\"), a RGB code (#FF0000 or #F00) or a 256-color-mode from the\n+ * terminal.\n */\n int color_parse(const char *value, char *dst);\n int color_parse_mem(const char *value, int len, char *dst);\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex c41138031989..b05f2a9b6075 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -96,8 +96,8 @@ test_expect_success '256 colors' '\n \tcolor \"254 bold 255\" \"[1;38;5;254;48;5;255m\"\n '\n \n-test_expect_success '24-bit colors' '\n-\tcolor \"#ff00ff black\" \"[38;2;255;0;255;40m\"\n+test_expect_success 'RGB colors' '\n+\tcolor \"#ff00ff #0f0\" \"[38;2;255;0;255;48;2;0;255;0m\"\n '\n \n test_expect_success '\"default\" foreground' '\n@@ -146,13 +146,15 @@ test_expect_success 'non-hex character in RGB color' '\n \tinvalid_color \"#12x456\" &&\n \tinvalid_color \"#123x56\" &&\n \tinvalid_color \"#1234x6\" &&\n-\tinvalid_color \"#12345x\"\n+\tinvalid_color \"#12345x\" &&\n+\tinvalid_color \"#x23\" &&\n+\tinvalid_color \"#1x3\" &&\n+\tinvalid_color \"#12x\"\n '\n \n test_expect_success 'wrong number of letters in RGB color' '\n \tinvalid_color \"#1\" &&\n \tinvalid_color \"#23\" &&\n-\tinvalid_color \"#456\" &&\n \tinvalid_color \"#789a\" &&\n \tinvalid_color \"#bcdef\" &&\n \tinvalid_color \"#1234567\"\n-- \n2.44.0\n\n"},{"id":"493921","messageId":"xmqqh6fgfa81.fsf@gitster.g","threadId":"61407","inReplyTo":"20240502110331.6347-1-dev+git@drbeat.li","subject":"Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-02T16:29:50Z","receivedAt":"2024-05-02T16:29:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Beat Bolli\" writes:\n\n> * The color parsing code learned to handle 12-bit RGB colors.\n>\n> The first commit fixes a typo, the second one adds some test coverage\n> for invalid RGB colors, and the final one extends the RGB color parser\n> to recognize 12-bit colors, as in #f0f.\n\nThanks.\n"},{"id":"494032","messageId":"20240503174841.GF3631237@coredump.intra.peff.net","threadId":"61407","inReplyTo":"20240502110331.6347-1-dev+git@drbeat.li","subject":"Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-03T17:48:41Z","receivedAt":"2024-05-03T17:48:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 02, 2024 at 01:03:28PM +0200, Beat Bolli wrote:\n\n> * The color parsing code learned to handle 12-bit RGB colors.\n> \n> The first commit fixes a typo, the second one adds some test coverage\n> for invalid RGB colors, and the final one extends the RGB color parser\n> to recognize 12-bit colors, as in #f0f.\n> ---\n> \n> Changes against v1:\n> - add test coverage for invalid RGB color lengths\n\nAh, I missed that you had sent a new version when I responded to Junio.\nYeah, this whole thing looks good to me!\n\n-Peff\n"},{"id":"494052","messageId":"6f421704-774f-4069-af18-7e4b8d7b42e9@drbeat.li","threadId":"61407","inReplyTo":"20240503174841.GF3631237@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/3] color: add support for 12-bit RGB colors","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2024-05-03T19:31:32Z","receivedAt":"2024-05-03T19:31:42Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 03.05.2024 19:48, Jeff King wrote:\n> On Thu, May 02, 2024 at 01:03:28PM +0200, Beat Bolli wrote:\n> \n>> * The color parsing code learned to handle 12-bit RGB colors.\n>>\n>> The first commit fixes a typo, the second one adds some test coverage\n>> for invalid RGB colors, and the final one extends the RGB color parser\n>> to recognize 12-bit colors, as in #f0f.\n>> ---\n>>\n>> Changes against v1:\n>> - add test coverage for invalid RGB color lengths\n> \n> Ah, I missed that you had sent a new version when I responded to Junio.\n> Yeah, this whole thing looks good to me!\n\nAll good ;-)\n\nThanks for reviewing!\n\nCheers, Beat\n\n"}]}