[PATCH v3] iproute2: color: default to dark background

[PATCH v3] iproute2: color: default to dark background

[Gedalya Nie]

Since the COLORFGBG environment variable isn't always there, and
anyway it seems that terminals and consoles more commonly default
to dark backgrounds, make that assumption here.

Currently the iproute2 tools produce output that is hard to read
when color is enabled and the background is dark.

Signed-off-by: Gedalya Nie <gedalya@gedalya.net>
---
 lib/color.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/color.c b/lib/color.c
index cd0f9f75..6692f9c1 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -72,7 +72,7 @@ static enum color attr_colors_dark[] = {
 	C_CLEAR
 };
 
-static int is_dark_bg;
+static int is_dark_bg = 1;
 static int color_is_enabled;
 
 static void enable_color(void)
@@ -127,11 +127,11 @@ static void set_color_palette(void)
 	 * COLORFGBG environment variable usually contains either two or three
 	 * values separated by semicolons; we want the last value in either case.
 	 * If this value is 0-6 or 8, background is dark.
+	 * If it is 7, 9 or greater, background is light.
 	 */
 	if (p && (p = strrchr(p, ';')) != NULL
-		&& ((p[1] >= '0' && p[1] <= '6') || p[1] == '8')
-		&& p[2] == '\0')
-		is_dark_bg = 1;
+		&& (p[1] == '7' || p[1] == '9' || p[2] != '\0'))
+		is_dark_bg = 0;
 }
 
 __attribute__((format(printf, 3, 4)))
-- 
2.43.0

Re: [PATCH v3] iproute2: color: default to dark background

[David Ahern]

On 5/22/24 12:43 PM, Gedalya Nie wrote:
> Since the COLORFGBG environment variable isn't always there, and
> anyway it seems that terminals and consoles more commonly default
> to dark backgrounds, make that assumption here.

Huge assumption. For example, I have one setup that defaults to dark
mode and another that defaults to light mode.

> 
> Currently the iproute2 tools produce output that is hard to read
> when color is enabled and the background is dark.

I agree with that statement, but the tools need to figure out the dark
vs light and adjust. We can't play games and guess what the right
default is.



--
pw-bot: reject

Re: [PATCH v3] iproute2: color: default to dark background

[Gedalya]

On 5/30/24 12:23 AM, David Ahern wrote:

> On 5/22/24 12:43 PM, Gedalya Nie wrote:
>> Since the COLORFGBG environment variable isn't always there, and
>> anyway it seems that terminals and consoles more commonly default
>> to dark backgrounds, make that assumption here.
> Huge assumption. For example, I have one setup that defaults to dark
> mode and another that defaults to light mode.

The code currently assumes light mode and it's generating complaints. It seems like we need to figure out a way to find some support for whatever is the best assumption.

>> Currently the iproute2 tools produce output that is hard to read
>> when color is enabled and the background is dark.
> I agree with that statement, but the tools need to figure out the dark
> vs light and adjust. We can't play games and guess what the right
> default is.
>
That's not possible.

COLORFGBG won't be allowed through by SSH servers.

If you try to write \e]11;?\a to the PTY you need to establish a timeout. There won't always be a response.
I'm not aware of any good way to do this, though I'm certainly not an expert. But I don't think that tools "figuring out dark vs light and adjusting" is a thing. If you just so happen to be happy with your results then so was I until Debian changed the way they build iproute2 and I never even used color overrides -- now I do. Tools just throw colors in your face and no, there is no really good and universally working way to be smart about it.
The fact remains that the code currently makes an assumption and I don't see why it is better than the other way around.
We need some kind of (possibly crude) way to assess what is more common, light or dark. But as I have pointed out already, as long as graphical terminal emulators are concerned, the reality out there is that people use themes and such and the ANSI color codes don't dictate the actual color displayed. But on a linux vt it is easier to say that the background will be dark, and it's neither simple to change the background nor to override the way ANSI colors are displayed.

Re: [PATCH v3] iproute2: color: default to dark background

[Edward Cree]

On 29/05/2024 17:36, Gedalya wrote:
> That's not possible.

Then in cases where the tool can't determine the answer, it
 should not be using colour at all unless explicitly instructed
 to do so, either on the command-line or in runtime
 configuration like dotfiles, /etc, or envvars.
"In the face of ambiguity, refuse the temptation to guess."[1]

> The fact remains that the code currently makes an
> assumption and I don't see why it is better than
> the other way around.

Because changing it would break it for people for whom it
 currently works.  That's a regression, and regressions are
 worse than existing bugs, ceteris paribus.
"Assess[ing] what is more common" won't change that.

-ed

[1]: https://peps.python.org/pep-0020/#the-zen-of-python

Re: [PATCH v3] iproute2: color: default to dark background

[Stephen Hemminger]

On Wed, 29 May 2024 18:51:01 +0100
Edward Cree <ecree.xilinx@gmail.com> wrote:

> On 29/05/2024 17:36, Gedalya wrote:
> > That's not possible.  
> 
> Then in cases where the tool can't determine the answer, it
>  should not be using colour at all unless explicitly instructed
>  to do so, either on the command-line or in runtime
>  configuration like dotfiles, /etc, or envvars.
> "In the face of ambiguity, refuse the temptation to guess."[1]
> 
> > The fact remains that the code currently makes an
> > assumption and I don't see why it is better than
> > the other way around.  
> 
> Because changing it would break it for people for whom it
>  currently works.  That's a regression, and regressions are
>  worse than existing bugs, ceteris paribus.
> "Assess[ing] what is more common" won't change that.
> 
> -ed
> 
> [1]: https://peps.python.org/pep-0020/#the-zen-of-python
> 

Debian maintainer decided to enable color by changing source.
Therefore, they should carry whatever color fixups they want as well.

Upstream will stay as default no color.

Re: [PATCH v3] iproute2: color: default to dark background

[Dragan Simic]

On 2024-05-29 18:36, Gedalya wrote:
> On 5/30/24 12:23 AM, David Ahern wrote:
> 
>> On 5/22/24 12:43 PM, Gedalya Nie wrote:
>>> Since the COLORFGBG environment variable isn't always there, and
>>> anyway it seems that terminals and consoles more commonly default
>>> to dark backgrounds, make that assumption here.
>> Huge assumption. For example, I have one setup that defaults to dark
>> mode and another that defaults to light mode.
> 
> The code currently assumes light mode and it's generating complaints.
> It seems like we need to figure out a way to find some support for
> whatever is the best assumption.

I think it would be best to modify the logic to require the presence
of COLORFGBG in the environment to enable colored output, if requested
by the user, or if built to be enabled by default.

>>> Currently the iproute2 tools produce output that is hard to read
>>> when color is enabled and the background is dark.
>> I agree with that statement, but the tools need to figure out the dark
>> vs light and adjust. We can't play games and guess what the right
>> default is.
>> 
> That's not possible.
> 
> COLORFGBG won't be allowed through by SSH servers.
> 
> If you try to write \e]11;?\a to the PTY you need to establish a
> timeout. There won't always be a response.
> I'm not aware of any good way to do this, though I'm certainly not an
> expert. But I don't think that tools "figuring out dark vs light and
> adjusting" is a thing. If you just so happen to be happy with your
> results then so was I until Debian changed the way they build iproute2
> and I never even used color overrides -- now I do. Tools just throw
> colors in your face and no, there is no really good and universally
> working way to be smart about it.
> The fact remains that the code currently makes an assumption and I
> don't see why it is better than the other way around.
> We need some kind of (possibly crude) way to assess what is more
> common, light or dark. But as I have pointed out already, as long as
> graphical terminal emulators are concerned, the reality out there is
> that people use themes and such and the ANSI color codes don't dictate
> the actual color displayed. But on a linux vt it is easier to say that
> the background will be dark, and it's neither simple to change the
> background nor to override the way ANSI colors are displayed.

Re: [PATCH v3] iproute2: color: default to dark background

[Stephen Hemminger]

On Wed, 29 May 2024 10:23:37 -0600
David Ahern <dsahern@kernel.org> wrote:

> On 5/22/24 12:43 PM, Gedalya Nie wrote:
> > Since the COLORFGBG environment variable isn't always there, and
> > anyway it seems that terminals and consoles more commonly default
> > to dark backgrounds, make that assumption here.  
> 
> Huge assumption. For example, I have one setup that defaults to dark
> mode and another that defaults to light mode.

Ditto.

Often use dark mode for VM's etc.