[2/2] color: support "default" to restore fg/bg color

Commit Message

Robert Estelle Oct. 25, 2021, 10:32 p.m. UTC

From: Robert Estelle <[e-mail]>

The name "default" can now be used in foreground or background colors,
and means to use the terminal's default color, discarding any
explicitly-set color without affecting the other attributes. On many
modern terminals, this is *not* the same as specifying "white" or
"black".

Although attributes could previously be cleared like "no-bold", there
had not been a similar mechanism available for colors, other than a full
"reset", which cannot currently be combined with other settings.

Note that this is *not* the same as the existing name "normal", which is
a no-op placeholder to permit setting the background without changing
the foreground. (i.e. what is currently called "normal" might have been
more descriptively named "inherit", "none", "pass" or similar).

Signed-off-by: Robert Estelle <[e-mail]>
---
 Documentation/config.txt | 18 +++++++++++++-----
 color.c                  | 23 ++++++++++++++++++++++-
 color.h                  |  4 ++++
 t/t4026-color.sh         | 12 ++++++++++++
 4 files changed, 51 insertions(+), 6 deletions(-)

Comments

brian m. carlson Oct. 25, 2021, 10:39 p.m. UTC | #1

On 2021-10-25 at 22:32:36, Robert Estelle via GitGitGadget wrote:
> From: Robert Estelle <[e-mail]>
> 
> The name "default" can now be used in foreground or background colors,
> and means to use the terminal's default color, discarding any
> explicitly-set color without affecting the other attributes. On many
> modern terminals, this is *not* the same as specifying "white" or
> "black".
> 
> Although attributes could previously be cleared like "no-bold", there
> had not been a similar mechanism available for colors, other than a full
> "reset", which cannot currently be combined with other settings.
> 
> Note that this is *not* the same as the existing name "normal", which is
> a no-op placeholder to permit setting the background without changing
> the foreground. (i.e. what is currently called "normal" might have been
> more descriptively named "inherit", "none", "pass" or similar).

I can't speak to whether this is correct, although you did mention the
standard, so I assume it is, but I do think this is a good feature to
have and the patch seems sane to me.  Patch 2 also looked good here.

For an example of why this differs from white on black, let me mention
that I use a semi-transparent terminal, where a black background is
opaque black, and the default is semi-transparent.  I assume other
possibilities include patterned backgrounds (Enlightenment, anyone?).
Whether you want to include something to this effect in the commit
message is up to you, but I provide it for the interested reader.

Junio C Hamano Oct. 25, 2021, 10:49 p.m. UTC | #2

"brian m. carlson" <[e-mail]> writes:

> For an example of why this differs from white on black, let me mention
> that I use a semi-transparent terminal, where a black background is
> opaque black, and the default is semi-transparent.  I assume other
> possibilities include patterned backgrounds (Enlightenment, anyone?).
> Whether you want to include something to this effect in the commit
> message is up to you, but I provide it for the interested reader.

It would help support the description of the cause, I would think.

I am also OK with these changes.  [1/2] is good for completeness,
and being able to say "go back to the default" without having to
know what the default is with [2/2] is also a good change.

Thanks, both.

Robert Estelle Oct. 26, 2021, 12:54 a.m. UTC | #3

Those are good examples. Note that this applies to more cases than
those relying on transparency or patterns: color terms often have
defaults that are not identical to their ANSI palette. I suspect
*most* themes are probably this way, based on a quick sampling.

You can see a bunch of examples here:
https://github.com/mbadolato/iTerm2-Color-Schemes#screenshots in all
the themes where the background color in the "40m" column isn't the
same as the unset column before it. (All of Terminal.app's built-in
themes are that way; I know xterm defaults to black on white vs white
on black; and so on).

Best,
Robert

On Mon, Oct 25, 2021 at 3:49 PM Junio C Hamano <[e-mail]> wrote:
>
> "brian m. carlson" <[e-mail]> writes:
>
> > For an example of why this differs from white on black, let me mention
> > that I use a semi-transparent terminal, where a black background is
> > opaque black, and the default is semi-transparent.  I assume other
> > possibilities include patterned backgrounds (Enlightenment, anyone?).
> > Whether you want to include something to this effect in the commit
> > message is up to you, but I provide it for the interested reader.
>
> It would help support the description of the cause, I would think.
>
> I am also OK with these changes.  [1/2] is good for completeness,
> and being able to say "go back to the default" without having to
> know what the default is with [2/2] is also a good change.
>
> Thanks, both.

Junio C Hamano Oct. 28, 2021, 4:41 p.m. UTC | #4

Robert Estelle <[e-mail]> writes:

> Those are good examples. Note that this applies to more cases than
> those relying on transparency or patterns: color terms often have
> defaults that are not identical to their ANSI palette. I suspect
> *most* themes are probably this way, based on a quick sampling.
>
> You can see a bunch of examples here:
> https://github.com/mbadolato/iTerm2-Color-Schemes#screenshots in all
> the themes where the background color in the "40m" column isn't the
> same as the unset column before it. (All of Terminal.app's built-in
> themes are that way; I know xterm defaults to black on white vs white
> on black; and so on).

I think you are wasting your time giving these to _us_ on the list
who happen to already understand what the issue you are attempting
to fix with these patches.  The suggestion was that you would help
future readers of "git log" if you wrote even just one or a few of
them in your proposed log message.  Those who will be digging the
history, perhaps they want to fix something in near-by code, may not
be aware of this discussion but the log message is a good place to
leave a note for them.

Thanks.

Robert Estelle Oct. 28, 2021, 5:33 p.m. UTC | #5

Sure. I can update the commit message. Both the commit message and the doc update already described that the default is terminal dependent and frequently not white on black, and the doc update gave transparent as an example. I expected that to be clear enough, but I can add more background. 

> On Oct 28, 2021, at 9:41 AM, Junio C Hamano <[e-mail]> wrote:
> 
> Robert Estelle <[e-mail]> writes:
> 
>> Those are good examples. Note that this applies to more cases than
>> those relying on transparency or patterns: color terms often have
>> defaults that are not identical to their ANSI palette. I suspect
>> *most* themes are probably this way, based on a quick sampling.
>> You can see a bunch of examples here:
>> https://github.com/mbadolato/iTerm2-Color-Schemes#screenshots in all
>> the themes where the background color in the "40m" column isn't the
>> same as the unset column before it. (All of Terminal.app's built-in
>> themes are that way; I know xterm defaults to black on white vs white
>> on black; and so on).
> 
> I think you are wasting your time giving these to _us_ on the list
> who happen to already understand what the issue you are attempting
> to fix with these patches.  The suggestion was that you would help
> future readers of "git log" if you wrote even just one or a few of
> them in your proposed log message.  Those who will be digging the
> history, perhaps they want to fix something in near-by code, may not
> be aware of this discussion but the log message is a good place to
> leave a note for them.
> 
> Thanks.

Patch

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf82766a6a2..78f13f061e5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -262,11 +262,19 @@  color::
        colors (at most two, one for foreground and one for background)
        and attributes (as many as you want), separated by spaces.
 +
-The basic colors accepted are `normal`, `black`, `red`, `green`, `yellow`,
-`blue`, `magenta`, `cyan` and `white`.  The first color given is the
-foreground; the second is the background.  All the basic colors except
-`normal` have a bright variant that can be specified by prefixing the
-color with `bright`, like `brightred`.
+The basic colors accepted are `normal`, `black`, `red`, `green`,
+`yellow`, `blue`, `magenta`, `cyan`, `white` and `default`.  The first
+color given is the foreground; the second is the background.  All the
+basic colors except `normal` and `default` have a bright variant that can
+be specified by prefixing the color with `bright`, like `brightred`.
++
+The color `normal` makes no change to the color. It is the same as an
+empty string, but can be used as the foreground color when specifying a
+background color alone (for example, "normal red").
++
+The color `default` explicitly resets the color to the terminal default,
+for example to specify a cleared background. Although it varies between
+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
diff --git a/color.c b/color.c
index 64f52a4f93a..a5fa9b79a7a 100644
--- a/color.c
+++ b/color.c
@@ -40,7 +40,7 @@  struct color {
 	enum {
 		COLOR_UNSPECIFIED = 0,
 		COLOR_NORMAL,
-		COLOR_ANSI, /* basic 0-7 ANSI colors */
+		COLOR_ANSI, /* basic 0-7 ANSI colors + "default" (value = 9) */
 		COLOR_256,
 		COLOR_RGB
 	} type;
@@ -83,6 +83,27 @@  static int parse_ansi_color(struct color *out, const char *name, int len)
 	int i;
 	int color_offset = COLOR_FOREGROUND_ANSI;
 
+	if (match_word(name, len, "default")) {
+		/*
+		 * Restores to the terminal's default color, which may not be
+		 * the same as explicitly setting "white" or "black".
+		 *
+		 * ECMA-48 - Control Functions \
+		 *  for Coded Character Sets, 5th edition (June 1991):
+		 * > 39 default display colour (implementation-defined)
+		 * > 49 default background colour (implementation-defined)
+		 *
+		 * Although not supported /everywhere/--according to terminfo,
+		 * some terminals define "op" (original pair) as a blunt
+		 * "set to white on black", or even "send full SGR reset"--
+		 * it's standard and well-supported enough that if a user
+		 * asks for it in their config this will do the right thing.
+		 */
+		out->type = COLOR_ANSI;
+		out->value = 9 + color_offset;
+		return 0;
+	}
+
 	if (strncasecmp(name, "bright", 6) == 0) {
 		color_offset = COLOR_FOREGROUND_BRIGHT_ANSI;
 		name += 6;
diff --git a/color.h b/color.h
index c20d324e7ca..27e817016bf 100644
--- a/color.h
+++ b/color.h
@@ -32,6 +32,7 @@  struct strbuf;
 #define GIT_COLOR_MAGENTA	"\033[35m"
 #define GIT_COLOR_CYAN		"\033[36m"
 #define GIT_COLOR_WHITE		"\033[37m"
+#define GIT_COLOR_DEFAULT	"\033[39m"
 #define GIT_COLOR_BOLD_BLACK	"\033[1;30m"
 #define GIT_COLOR_BOLD_RED	"\033[1;31m"
 #define GIT_COLOR_BOLD_GREEN	"\033[1;32m"
@@ -40,6 +41,7 @@  struct strbuf;
 #define GIT_COLOR_BOLD_MAGENTA	"\033[1;35m"
 #define GIT_COLOR_BOLD_CYAN	"\033[1;36m"
 #define GIT_COLOR_BOLD_WHITE	"\033[1;37m"
+#define GIT_COLOR_BOLD_DEFAULT	"\033[1;39m"
 #define GIT_COLOR_FAINT_BLACK	"\033[2;30m"
 #define GIT_COLOR_FAINT_RED	"\033[2;31m"
 #define GIT_COLOR_FAINT_GREEN	"\033[2;32m"
@@ -48,6 +50,7 @@  struct strbuf;
 #define GIT_COLOR_FAINT_MAGENTA	"\033[2;35m"
 #define GIT_COLOR_FAINT_CYAN	"\033[2;36m"
 #define GIT_COLOR_FAINT_WHITE	"\033[2;37m"
+#define GIT_COLOR_FAINT_DEFAULT	"\033[2;39m"
 #define GIT_COLOR_BG_BLACK	"\033[40m"
 #define GIT_COLOR_BG_RED	"\033[41m"
 #define GIT_COLOR_BG_GREEN	"\033[42m"
@@ -56,6 +59,7 @@  struct strbuf;
 #define GIT_COLOR_BG_MAGENTA	"\033[45m"
 #define GIT_COLOR_BG_CYAN	"\033[46m"
 #define GIT_COLOR_BG_WHITE	"\033[47m"
+#define GIT_COLOR_BG_DEFAULT	"\033[49m"
 #define GIT_COLOR_FAINT		"\033[2m"
 #define GIT_COLOR_FAINT_ITALIC	"\033[2;3m"
 #define GIT_COLOR_REVERSE	"\033[7m"
diff --git a/t/t4026-color.sh b/t/t4026-color.sh
index c0b642c1ab0..e34838ded95 100755
--- a/t/t4026-color.sh
+++ b/t/t4026-color.sh
@@ -94,6 +94,18 @@  test_expect_success '24-bit colors' '
 	color "#ff00ff black" "[38;2;255;0;255;40m"
 '
 
+test_expect_success '"default" foreground' '
+	color "default" "[39m"
+'
+
+test_expect_success '"normal default" to clear background' '
+	color "normal default" "[49m"
+'
+
+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"' '
 	color "normal black" "[40m"
 '