Re: [PATCH] dircolors: consider COLORTERM sufficient for color

2022-02-14 Thread Pádraig Brady

On 14/02/2022 23:36, Arsen Arsenović wrote:

[ Reposting since I didn't reply-all the first time; apologies ]


Your change could break this setup if there were
entries in "specific config" that weren't overridden later,
and even if they were, it would result in larger LS_COLORS env vars.

Ah, I see. I lacked a full understanding of this format so I overlooked
that, sorry.


How about the attached patch to allow matching COLORTERM
just like TERM, and set the pattern in the default config
to match against any COLORTERM value.
This also has the advantage of handling specific COLORTERM values.

This looks fine, and works on my machine.


We would have to add the default COLORTERM entry to distro config
(when supported by the installed dircolors), but that should be easy enough to 
do.

Don't distros just generally use the compiled-in default?


Well they have variants generally in /etc for 256 colors etc.
I'll handle updating the Fedora one at least.

cheers,
Pádraig



Re: [PATCH] dircolors: consider COLORTERM sufficient for color

2022-02-14 Thread Arsen Arsenović
[ Reposting since I didn't reply-all the first time; apologies ]

> Your change could break this setup if there were
> entries in "specific config" that weren't overridden later,
> and even if they were, it would result in larger LS_COLORS env vars.
Ah, I see. I lacked a full understanding of this format so I overlooked
that, sorry.

> How about the attached patch to allow matching COLORTERM
> just like TERM, and set the pattern in the default config
> to match against any COLORTERM value.
> This also has the advantage of handling specific COLORTERM values.
This looks fine, and works on my machine.

> We would have to add the default COLORTERM entry to distro config
> (when supported by the installed dircolors), but that should be easy enough 
> to do.
Don't distros just generally use the compiled-in default?

Thanks again,
-- 
Arsen Arsenović


signature.asc
Description: PGP signature


Re: [PATCH] dircolors: consider COLORTERM sufficient for color

2022-02-14 Thread Pádraig Brady

On 12/02/2022 21:54, Arsen Arsenović wrote:

COLORTERM is an environment used usually to expose truecolor support in
terminal emulators. If a terminal emulator supports truecolor, it is
surely reasonable to assume it also supports 8/16/256 colors.

This implicitly supports foot, alacritty and any other truecolor
terminal emulator with unmatched $TERM.
---
Good evening,

I've noticed dircolors does not work in foot and alacritty, and on previously
raised patches about adding a TERM entry for them the concern of a nongeneric
solution was brought up. This is a valid concern, and as many terminals
(including these two) export COLORTERM to advertise 24-bit color support, it'd
appear to be a good way to pick up on color support.


COLORTERM does seem to be a way to advertise truecolor support:
https://gitlab.com/gnachman/iterm2/-/issues/5294

Note dircolors supports config files for multiple terminals like:

  global config
  ...
  TERM should_not_match
  specific config
  ...
  TERM should_match
  ...

Your change could break this setup if there were
entries in "specific config" that weren't overridden later,
and even if they were, it would result in larger LS_COLORS env vars.

How about the attached patch to allow matching COLORTERM
just like TERM, and set the pattern in the default config
to match against any COLORTERM value.
This also has the advantage of handling specific COLORTERM values.
We would have to add the default COLORTERM entry to distro config
(when supported by the installed dircolors), but that should be easy enough to 
do.

I've also attached a patch to avoid needless calls to fnmatch().

cheers,
PádraigFrom 9f9a9fccb836b5933d4479d0eb274a80ffe5ad37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Mon, 14 Feb 2022 17:25:04 +
Subject: [PATCH 2/2] dircolors: speed up processing of TERM entries

* src/dircolors.c (main): Avoid glob matching
when we've already matched in a group of {COLOR,}TERM entries.
---
 src/dircolors.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/dircolors.c b/src/dircolors.c
index da0e1aed3..b543e8568 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -328,17 +328,13 @@ dc_parse_stream (FILE *fp, char const *filename)
   unrecognized = false;
   if (c_strcasecmp (keywd, "TERM") == 0)
 {
-  if (fnmatch (arg, term, 0) == 0)
-state = ST_TERMSURE;
-  else if (state != ST_TERMSURE)
-state = ST_TERMNO;
+  if (state != ST_TERMSURE)
+state = fnmatch (arg, term, 0) == 0 ? ST_TERMSURE : ST_TERMNO;
 }
   else if (c_strcasecmp (keywd, "COLORTERM") == 0)
 {
-  if (fnmatch (arg, colorterm, 0) == 0)
-state = ST_TERMSURE;
-  else if (state != ST_TERMSURE)
-state = ST_TERMNO;
+  if (state != ST_TERMSURE)
+state = fnmatch (arg, colorterm, 0) == 0 ? ST_TERMSURE : ST_TERMNO;
 }
   else
 {
-- 
2.26.2

From b8472a9dbe91ce577d71c098eb947b9425053905 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 12 Feb 2022 22:54:07 +0100
Subject: [PATCH 1/2] dircolors: consider COLORTERM as well as TERM env vars

COLORTERM is an environment used usually to expose truecolor support in
terminal emulators.  Therefore support matches on that in addition
to TERM.  Also set the default COLORTERM match pattern so that
we apply colors if COLORTERM is any value.

This implicitly supports a terminal like "foot"
without a need for an explicit TERM entry.

* NEWS: Mention the new feature.
* src/dircolors.c (main): Match COLORTERM like we do for TERM.
* src/dircolors.hin: Add default config to match any COLORTERM.
* tests/misc/dircolors.pl: Add test cases.
---
 NEWS|  3 +++
 src/dircolors.c | 15 ++-
 src/dircolors.hin   | 10 --
 tests/misc/dircolors.pl |  8 
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index e14501e77..ef65b4ab8 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,9 @@ GNU coreutils NEWS-*- outline -*-
   dircolors takes a new --print-ls-colors option to display LS_COLORS
   entries, on separate lines, colored according to the entry color code.
 
+  dircolors will now also match COLORTERM in addition to TERM environment
+  variables.  The default config will apply colors with any COLORTERM set.
+
 ** Improvements
 
   cp, mv, and install now use openat-like syscalls when copying to a directory.
diff --git a/src/dircolors.c b/src/dircolors.c
index 8bb4abfc4..da0e1aed3 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -271,6 +271,7 @@ dc_parse_stream (FILE *fp, char const *filename)
   size_t input_line_size = 0;
   char const *line;
   char const *term;
+  char const *colorterm;
   bool ok = true;
 
   /* State for the parser.  */
@@ -281,6 +282,11 @@ dc_parse_stream (FILE *fp, char const *filename)
   if (term == NULL || 

Re: [PATCH] dircolors: consider COLORTERM sufficient for color

2022-02-13 Thread Arsen Arsenović
On 22/02/13 14:15, Pádraig Brady wrote:
> Though it was pointed out that COLORTERM may not be preserved across ssh etc.
> Now doing this would be no worse, so I'm inclined to apply this.
I could follow up with OpenSSH at some point, too, to include $COLORTERM
in the default environment set. Though, operating over ssh also presents
other considerable problems, such as the remote side lacking the
matching terminfo (at least by default), which is why I mostly
overlooked it.

> BTW alacritty was just added as an explicitly matched terminal.
Ah, nice; I didn't realize.
-- 
Arsen Arsenović


signature.asc
Description: PGP signature


Re: [PATCH] dircolors: consider COLORTERM sufficient for color

2022-02-13 Thread Pádraig Brady

On 12/02/2022 21:54, Arsen Arsenović wrote:

COLORTERM is an environment used usually to expose truecolor support in
terminal emulators. If a terminal emulator supports truecolor, it is
surely reasonable to assume it also supports 8/16/256 colors.

This implicitly supports foot, alacritty and any other truecolor
terminal emulator with unmatched $TERM.
---
Good evening,

I've noticed dircolors does not work in foot and alacritty, and on previously
raised patches about adding a TERM entry for them the concern of a nongeneric
solution was brought up. This is a valid concern, and as many terminals
(including these two) export COLORTERM to advertise 24-bit color support, it'd
appear to be a good way to pick up on color support.


I did suggest this at:
https://lists.gnu.org/archive/html/coreutils/2021-10/msg8.html
Though it was pointed out that COLORTERM may not be preserved across ssh etc.
Now doing this would be no worse, so I'm inclined to apply this.

BTW alacritty was just added as an explicitly matched terminal.

cheers,
Pádraig