{"thread":{"id":"56784","subject":"[PATCH] color: allow colors to be prefixed with \"reset\"","startedAt":"2021-10-26T01:03:52Z","lastAt":"2021-10-27T04:29:17Z","messageCount":3,"participants":["Robert Estelle via GitGitGadget","Junio C Hamano","Robert Estelle"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"439617","messageId":"pull.1117.git.git.1635210227532.gitgitgadget@gmail.com","threadId":"56784","inReplyTo":null,"subject":"[PATCH] color: allow colors to be prefixed with \"reset\"","fromName":"Robert Estelle via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-10-26T01:03:47Z","receivedAt":"2021-10-26T01:03:52Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"From: Robert Estelle <robertestelle@gmail.com>\n\n\"reset\" was previously treated as a standalone special color name\nrepresenting `\\e[m`. Now, it can apply to other color properties,\nallowing exact specifications without implicit attribute inheritance.\n\nFor example, \"reset green\" now renders `\\e[;32m`, which is interpreted\nas \"reset everything; then set foreground to green\". This means the\nbackground and other attributes are also reset to their defaults.\n\nPreviously, this was impossible to represent in a single color:\n\"reset\" could be specified alone, or a color with attributes, but some\nthing like clearing a background color were impossible.\n\nThere is a separate change that introduces the \"default\" color name to\nassist with that, but even then, the above could only to be represented\nby explicitly disabling each of the attributes:\n  green default no-bold no-dim no-italic no-ul no-blink no-reverse no-strike\n\nSigned-off-by: Robert Estelle <robertestelle@gmail.com>\n---\n    color: allow color specs to be prefixed by \"reset\"\n    \n    The pseudo-attribute \"reset\", which was previously treated as a\n    standalone special color name, can now be combined with other color\n    properties.\n    \n    This allows specification of an \"entire\" text style without implicitly\n    inheriting existing properties (colors, bold/italic/etc) nor requiring a\n    comprehensive clearing of each of them.\n    \n    For example, \"reset green\" now renders \\e[;32m, which is interpreted as\n    \"default properties; then set foreground to green\".\n    \n    Previously, it was not possible to represent this in a single color due\n    to inability to clear the foreground or background (at best, they could\n    be overridden to a some explicit value). There is a complementary\n    proposed change (color: support \"default\" to restore fg/bg color,\n    https://lore.kernel.org/git/pull.1116.git.git.1635201156.gitgitgadget@gmail.com/,\n    https://github.com/git/git/pull/1116) which introduces the \"default\"\n    color name to facilitate that, but even then, the above would still\n    require disabling each of the attributes explicitly: green default\n    no-bold no-dim no-italic no-ul no-blink no-reverse no-strike\n    \n    This is useful in scenarios that the \"reset\" word or any of the \"no-\"\n    attribute modifiers might have been used before, e.g.\n    colors.diff-highlight.newReset.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1117%2Frwe%2Freset-color-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1117/rwe/reset-color-v1\nPull-Request: https://github.com/git/git/pull/1117\n\n Documentation/config.txt |  5 +++++\n color.c                  | 18 +++++++++++-------\n color.h                  |  1 +\n t/t4026-color.sh         |  4 ++++\n 4 files changed, 21 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bf82766a6a2..b1423b6ce8b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -280,6 +280,11 @@ The position of any attributes with respect to the colors\n be turned off by prefixing them with `no` or `no-` (e.g., `noreverse`,\n `no-ul`, etc).\n +\n+The pseudo-attribute `reset` resets all colors and attributes before\n+applying the specified coloring. For example, `reset green` will result\n+in a green foreground and default background without any active\n+attributes.\n++\n An empty color string produces no color effect at all. This can be used\n to avoid coloring specific elements without disabling color entirely.\n +\ndiff --git a/color.c b/color.c\nindex 64f52a4f93a..29ac83b2d6e 100644\n--- a/color.c\n+++ b/color.c\n@@ -234,6 +234,7 @@ int color_parse_mem(const char *value, int value_len, char *dst)\n \tconst char *ptr = value;\n \tint len = value_len;\n \tchar *end = dst + COLOR_MAXLEN;\n+\tunsigned int has_reset = 0;\n \tunsigned int attr = 0;\n \tstruct color fg = { COLOR_UNSPECIFIED };\n \tstruct color bg = { COLOR_UNSPECIFIED };\n@@ -248,12 +249,7 @@ int color_parse_mem(const char *value, int value_len, char *dst)\n \t\treturn 0;\n \t}\n \n-\tif (!strncasecmp(ptr, \"reset\", len)) {\n-\t\txsnprintf(dst, end - dst, GIT_COLOR_RESET);\n-\t\treturn 0;\n-\t}\n-\n-\t/* [fg [bg]] [attr]... */\n+\t/* [reset] [fg [bg]] [attr]... */\n \twhile (len > 0) {\n \t\tconst char *word = ptr;\n \t\tstruct color c = { COLOR_UNSPECIFIED };\n@@ -270,6 +266,11 @@ int color_parse_mem(const char *value, int value_len, char *dst)\n \t\t\tlen--;\n \t\t}\n \n+\t\tif (match_word(word, wordlen, \"reset\")) {\n+\t\t\thas_reset = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tif (!parse_color(&c, word, wordlen)) {\n \t\t\tif (fg.type == COLOR_UNSPECIFIED) {\n \t\t\t\tfg = c;\n@@ -295,13 +296,16 @@ int color_parse_mem(const char *value, int value_len, char *dst)\n \t*dst++ = (x); \\\n } while(0)\n \n-\tif (attr || !color_empty(&fg) || !color_empty(&bg)) {\n+\tif (has_reset || attr || !color_empty(&fg) || !color_empty(&bg)) {\n \t\tint sep = 0;\n \t\tint i;\n \n \t\tOUT('\\033');\n \t\tOUT('[');\n \n+\t\tif (has_reset)\n+\t\t\tsep++;\n+\n \t\tfor (i = 0; attr; i++) {\n \t\t\tunsigned bit = (1 << i);\n \t\t\tif (!(attr & bit))\ndiff --git a/color.h b/color.h\nindex 98894d6a175..684dbd3bceb 100644\n--- a/color.h\n+++ b/color.h\n@@ -6,6 +6,7 @@ struct strbuf;\n /*\n  * The maximum length of ANSI color sequence we would generate:\n  * - leading ESC '['            2\n+ * - reset ';' .................1\n  * - attr + ';'                 2 * num_attr (e.g. \"1;\")\n  * - no-attr + ';'              3 * num_attr (e.g. \"22;\")\n  * - fg color + ';'             17 (e.g. \"38;2;255;255;255;\")\ndiff --git a/t/t4026-color.sh b/t/t4026-color.sh\nindex c0b642c1ab0..5ef2ff28c48 100755\n--- a/t/t4026-color.sh\n+++ b/t/t4026-color.sh\n@@ -58,6 +58,10 @@ test_expect_success 'fg bg attr...' '\n \tcolor \"blue bold dim ul blink reverse\" \"[1;2;4;5;7;34m\"\n '\n \n+test_expect_success 'reset fg bg attr...' '\n+\tcolor \"reset blue bold dim ul blink reverse\" \"[;1;2;4;5;7;34m\"\n+'\n+\n # note that nobold and nodim are the same code (22)\n test_expect_success 'attr negation' '\n \tcolor \"nobold nodim noul noblink noreverse\" \"[22;24;25;27m\"\n\nbase-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf\n-- \ngitgitgadget\n"},{"id":"439690","messageId":"xmqqee87xy39.fsf@gitster.g","threadId":"56784","inReplyTo":"pull.1117.git.git.1635210227532.gitgitgadget@gmail.com","subject":"Re: [PATCH] color: allow colors to be prefixed with \"reset\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-26T21:42:02Z","receivedAt":"2021-10-26T21:42:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robert Estelle via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Robert Estelle <robertestelle@gmail.com>\n>\n> \"reset\" was previously treated as a standalone special color name\n> representing `\\e[m`. Now, it can apply to other color properties,\n> allowing exact specifications without implicit attribute inheritance.\n>\n> For example, \"reset green\" now renders `\\e[;32m`, which is interpreted\n> as \"reset everything; then set foreground to green\". This means the\n> background and other attributes are also reset to their defaults.\n>\n> Previously, this was impossible to represent in a single color:\n> \"reset\" could be specified alone, or a color with attributes, but some\n> thing like clearing a background color were impossible.\n>\n> There is a separate change that introduces the \"default\" color name to\n> assist with that, but even then, the above could only to be represented\n> by explicitly disabling each of the attributes:\n>   green default no-bold no-dim no-italic no-ul no-blink no-reverse no-strike\n>\n> Signed-off-by: Robert Estelle <robertestelle@gmail.com>\n> ---\n\nUnlike the \"default\" patch, I quite do not see the point of the\nexample(s).  Instead of saying \"reset green\", can't we already say\n\"set bg to default, and set fg to green\", thanks to the other one?\nOr does \"default\" do too little to deserve a name that implies \"go\nback to default\", e.g. by not defeating the 'blink' attribute that\nwas set earlier?\n"},{"id":"439709","messageId":"CAMmwyQpVHuo0zuCZ9YmSn4=J0njeRNF3ooxyZ5nggL3HRxx8fw@mail.gmail.com","threadId":"56784","inReplyTo":"xmqqee87xy39.fsf@gitster.g","subject":"Re: [PATCH] color: allow colors to be prefixed with \"reset\"","fromName":"Robert Estelle","fromEmail":"robertestelle@gmail.com","sentAt":"2021-10-27T04:28:48Z","receivedAt":"2021-10-27T04:29:17Z","isPatch":true,"sender":{"key":"robertestelle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/447615?v=4"},"body":"> Unlike the \"default\" patch, I quite do not see the point of the example(s).\n\nYeah. This \"reset\" one was developed mainly as a hedge in case there\nwere concerns about the \"default\" one. You're correct that the\n\"default\" one provides a finer-grained capability, and if we had to\nchoose, I'd go with that. However, the two are mutually compatible (no\nconflicts, even) and complementary.\n\nThis \"reset\" approach seemed easier to go through since it just\ngeneralizes the treatment of the existing \"reset\" capability without\nintroducing any new names or even SGR sequence numbers.\n\n(The immediate goal of was to have some way to control from a color\nwhether to inherit or remove an existing background).\n\n> Instead of saying \"reset green\", can't we already say \"set bg to default, and set fg to green\", thanks to the other one?\n\nYes-ish, however, \"default\" is just a color name: and like other\ncolors, it doesn't imply any attribute (bold etc) resets. Like, if you\nturn on bold, and then change the color to red, you'll get bold red.\nIf you then change the foreground back to default, you'll have bold\nwhatever.\n\nInstead, with \"default\", to avoid inheriting you could write:\n   green default no-bold no-dim no-italic no-ul no-blink no-reverse no-strike\nresulting in an ANSI sequence like: `\\e[22;23;24;25;27;29;32;49m`\n\nIt's just a mouthful. `reset green` results in `\\e[;32m`, which does\nfunctionally the same thing. It's just the combination of \"reset\neverything\" (``\\e[m` or equivalently \\e[0m`) followed by a color\n(`\\e[32m`).  I considered calling \"reset\" something like \"only\" since\nthat reads more clearly to me, but I also felt it was not worth the\ncuteness when \"reset\" already existed and was defined consistently.\n\n> Or does \"default\" do too little to deserve a name that implies \"go back to default\", e.g. by not defeating the 'blink' attribute that was set earlier?\n\nI agree that \"default\" feels ambiguous. Within the current\npositional-name format vs. e.g. \"fg=green bg=default bold=on\", I had a\nlot of trouble thinking of a name that wouldn't be. However, \"default\"\nis the same word as in the ANSI spec, used in console_codes(4) (see\nheading \"ECMA-48 Select Graphic Rendition here:\nhttps://man7.org/linux/man-pages/man4/console_codes.4.html) and\nncurses (https://man7.org/linux/man-pages/man3/default_colors.3x.html).\nI also explored a few terminals' settings to see if there was a better\nname: usually they'll just refer to it unhelpfully as \"foreground\" or\n\"background\", but when they're otherwise named it's usually like\n\"DefaultForeground\".\n\nOther names I'd considered had their own problems: \"unset\" could be\nconfusing alongside the existing \"reset\", \"clear\" could be confusing\nas a foreground or in the context of translucent terms, \"no-color\" is\nusually just untrue, and \"normal\" is (very confusingly, IMO) already\nspecial-cased as a no-op word.\n\nBest,\nRobert\n\nOn Tue, Oct 26, 2021 at 2:42 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Robert Estelle via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: Robert Estelle <robertestelle@gmail.com>\n> >\n> > \"reset\" was previously treated as a standalone special color name\n> > representing `\\e[m`. Now, it can apply to other color properties,\n> > allowing exact specifications without implicit attribute inheritance.\n> >\n> > For example, \"reset green\" now renders `\\e[;32m`, which is interpreted\n> > as \"reset everything; then set foreground to green\". This means the\n> > background and other attributes are also reset to their defaults.\n> >\n> > Previously, this was impossible to represent in a single color:\n> > \"reset\" could be specified alone, or a color with attributes, but some\n> > thing like clearing a background color were impossible.\n> >\n> > There is a separate change that introduces the \"default\" color name to\n> > assist with that, but even then, the above could only to be represented\n> > by explicitly disabling each of the attributes:\n> >   green default no-bold no-dim no-italic no-ul no-blink no-reverse no-strike\n> >\n> > Signed-off-by: Robert Estelle <robertestelle@gmail.com>\n> > ---\n>\n> Unlike the \"default\" patch, I quite do not see the point of the\n> example(s).  Instead of saying \"reset green\", can't we already say\n> \"set bg to default, and set fg to green\", thanks to the other one?\n> Or does \"default\" do too little to deserve a name that implies \"go\n> back to default\", e.g. by not defeating the 'blink' attribute that\n> was set earlier?\n"}]}