[PATCH] [resend] color: default to dark background

[PATCH] [resend] color: default to dark background

[Gedalya Nie]

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] [resend] color: default to dark background

[Stephen Hemminger]

On Thu, 23 May 2024 02:43:57 +0800
Gedalya Nie <gedalya@gedalya.net> wrote:

> Signed-off-by: Gedalya Nie <gedalya@gedalya.net>

Why? What other utilities do the same thin?

Re: [PATCH] [resend] color: default to dark background

[Gedalya]

On 5/23/24 4:57 AM, Stephen Hemminger wrote:
> Why? What other utilities do the same thin?

I'm truly sorry, I don't understand the question

Re: [PATCH] [resend] color: default to dark background

[Dragan Simic]

On 2024-05-22 23:01, Gedalya wrote:
> On 5/23/24 4:57 AM, Stephen Hemminger wrote:
>> Why? What other utilities do the same thin?
> 
> I'm truly sorry, I don't understand the question

Basically, you need to provide the patch description.

Re: [PATCH] [resend] color: default to dark background

[Stephen Hemminger]

On Wed, 22 May 2024 23:02:37 +0200
Dragan Simic <dsimic@manjaro.org> wrote:

> On 2024-05-22 23:01, Gedalya wrote:
> > On 5/23/24 4:57 AM, Stephen Hemminger wrote:  
> >> Why? What other utilities do the same thin?  
> > 
> > I'm truly sorry, I don't understand the question  
> 
> Basically, you need to provide the patch description.

The color handling of iproute2 was inherited from other utilities such as vim.
There doesn't appear to be any library or standardization, all this ad-hoc.

In current vim:

    char_u *
term_bg_default(void)
{
#if defined(MSWIN)
    // DOS console is nearly always black
    return (char_u *)"dark";
#else
    char_u	*p;

    if (STRCMP(T_NAME, "linux") == 0
	    || STRCMP(T_NAME, "screen.linux") == 0
	    || STRNCMP(T_NAME, "cygwin", 6) == 0
	    || STRNCMP(T_NAME, "putty", 5) == 0
	    || ((p = mch_getenv((char_u *)"COLORFGBG")) != NULL
		&& (p = vim_strrchr(p, ';')) != NULL
		&& ((p[1] >= '0' && p[1] <= '6') || p[1] == '8')
		&& p[2] == NUL))
	return (char_u *)"dark";
    return (char_u *)"light";
#endif
}

Re: [PATCH] [resend] color: default to dark background

[Gedalya]

On 5/23/24 5:33 AM, Stephen Hemminger wrote:

> The color handling of iproute2 was inherited from other utilities such as vim.
> There doesn't appear to be any library or standardization, all this ad-hoc.

Looking at the vim code, and playing around with the program, I
have a few observations.

The snippet you quoted isn't doing anything brilliant. It just
assumes that certain types of terminals are dark, regardless of
the implementation and configuration. All you can really say is
that terminals are often dark which is what I was saying here in
the first place.

I'm not seeing any justification for assuming dark in certain
cases and light otherwise. The code just happens to be that way.

More importantly, vim does happen to actually work. So far I was
only able to get it to show dark blue on a black background by
setting TERM=ansi.

The results are what is important. Vim has its own various color
palettes and it's a serious full-screen app, uses the terminfo
library, its support for terminals is much more complex than just
two palettes. One way or another, we need to fix this, probably
not by linking against ncurses, and "assuming terminal backgrounds
are light" isn't the nugget of wisdom vim has to offer.

Re: [PATCH] [resend] color: default to dark background

[Stephen Hemminger]

On Thu, 23 May 2024 06:11:56 +0800
Gedalya <gedalya@gedalya.net> wrote:

> On 5/23/24 5:33 AM, Stephen Hemminger wrote:
> > The color handling of iproute2 was inherited from other utilities such as vim.
> > There doesn't appear to be any library or standardization, all this ad-hoc.  
> 
> Looking at the vim code, and playing around with the program, I have a few observations.
> 
> The snippet you quoted isn't doing anything brilliant. It just assumes that certain types of terminals are dark, regardless of the implementation and configuration. All you can really say is that terminals are often dark which is what I was saying here in the first place.
> 
> I'm not seeing any justification for assuming dark in certain cases and light otherwise. The code just happens to be that way.
> 
> More importantly, vim does happen to actually work. So far I was only able to get it to show dark blue on a black background by setting TERM=ansi.
> 
> The results are what is important. Vim has its own various color palettes and it's a curses app, its support for terminals is much more complex than just two palettes. One way or another, we need to fix this, probably not by linking against ncurses, and "assuming terminal backgrounds are light" isn't the nugget of wisdom vim has to offer.
> 

Overall, I am concerned that changing this will upset existing users.
Not that it is impossible, just need more consensus and testing.

Re: [PATCH] [resend] color: default to dark background

[Gedalya]

On 5/23/24 6:52 AM, Stephen Hemminger wrote:

> Overall, I am concerned that changing this will upset existing users.
> Not that it is impossible, just need more consensus and testing.

Makes sense.
It seems that possible negative impact would be when the following are all true:

1. Users actually using color. For debian and derivatives, up until now this
would have been people enabling color themselves.
Debian's recent enabling color by default was the impetus for this issue.

2. Using light background.

3. Not using COLORFGBG, or setting it incorrectly.

Re: [PATCH] [resend] color: default to dark background

[Gedalya]

On 5/23/24 6:52 AM, Stephen Hemminger wrote:

> Overall, I am concerned that changing this will upset existing users.
> Not that it is impossible, just need more consensus and testing.

Turns out people were complaining about it years ago, not realizing that
the color scheme they were looking at was actually not meant for dark backgrounds:

https://www.reddit.com/r/linux/comments/kae9qa/comment/gfgew0x/

Re: [PATCH] [resend] color: default to dark background

[Dragan Simic]

On 2024-05-23 01:41, Gedalya wrote:
> On 5/23/24 6:52 AM, Stephen Hemminger wrote:
> 
>> Overall, I am concerned that changing this will upset existing users.
>> Not that it is impossible, just need more consensus and testing.
> 
> Turns out people were complaining about it years ago, not realizing 
> that
> the color scheme they were looking at was actually not meant for dark
> backgrounds:
> 
> https://www.reddit.com/r/linux/comments/kae9qa/comment/gfgew0x/

That's what I also wondered about after enabling the color support
for the first time, which wasn't very unusable with dark background.
After a bit of reading, I got it all configured properly.

Re: [PATCH] [resend] color: default to dark background

[Dragan Simic]

On 2024-05-23 02:12, Dragan Simic wrote:
> On 2024-05-23 01:41, Gedalya wrote:
>> On 5/23/24 6:52 AM, Stephen Hemminger wrote:
>> 
>>> Overall, I am concerned that changing this will upset existing users.
>>> Not that it is impossible, just need more consensus and testing.
>> 
>> Turns out people were complaining about it years ago, not realizing 
>> that
>> the color scheme they were looking at was actually not meant for dark
>> backgrounds:
>> 
>> https://www.reddit.com/r/linux/comments/kae9qa/comment/gfgew0x/
> 
> That's what I also wondered about after enabling the color support
> for the first time, which wasn't very unusable with dark background.
> After a bit of reading, I got it all configured properly.

Oops...  s/very unusable/very usable/
Sorry for the noise.