bug#44950: 28.0.50; 24-bit colors not used in terminal with emacsclient

[Tim Ruffing <crypto@timruffing.de>]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]

Sorry for committing to send an updating patch but not replying for so
long...

Good that the broken patch was quickly fixed after it broke colors in
the macOS Terminal app. I looked again into the issue. One potential
further pitfall is that the Tc logic in tmux, which introduced the Tc
flag, is still different from what we do: while Tc is meant to mean
"support for the default sequences as in xterm+direct", tmux first
relies on the setrgbf/setrgbb escape sequences in the terminfo and only
provide the default sequences if setrgbf/setrgbb are not present.
 
I've attached a patch that would introduce full support for
setrgbf/setrgbb as previously proposed [2]. Before I propose an update
for the efaq.texi, let me know if you're interested in this patch (with
a proper commit messagea and NEWS entry) or not.

I personally don't care too much. With the Tc fix applied, Emacs works
for me, and believe our current code is fine, even though we don't
fully replicate tmux's logic respect to Tc. On the other hand, with
full support for setrgbf/setrgbb, we'd support pretty much every
existing method out there and the diff is not that large.

Best,
Tim



[1] See for example
https://github.com/tmux/tmux/commit/7eb496c00c313c2f8ab8debe6d154d5ac0db277b#diff-de4f90e163caf6cc83476898c795355523776a76f9ccc7783e9bd3a99fde671dR526
(this code was replaced later in 
https://github.com/tmux/tmux/commit/a6129e99749d2bbc8b4a991c7b5d09300aa55f39#
but the logic is still the same and the earlier commit is easier to
read). Another relevant discussion is
https://github.com/tmux/tmux/issues/2418 .

[2] https://lists.gnu.org/archive/html/bug-ncurses/2013-10/msg00007.html



On Thu, 2021-11-11 at 07:15 +0100, Lars Ingebrigtsen wrote:
> Tim Ruffing <crypto@timruffing.de> writes:
> 
> > Hi Lars, not yet. I'm currently on vacation, I'll come back to you in
> > about two weeks.
> 
> This was some months ago, so I went ahead and pushed the change to
> Emacs
> 29.
> 

[-- Attachment #2: 0001-Support-setrgbb-setrgbf-for-setting-24-bit-color.patch --]
[-- Type: text/x-patch, Size: 3501 bytes --]

From c296de5778f040e5803326dbb74a7362ac0146fa Mon Sep 17 00:00:00 2001
From: Tim Ruffing <crypto@timruffing.de>
Date: Fri, 12 Nov 2021 20:37:39 +0100
Subject: [PATCH] Support setrgbb/setrgbf for setting 24-bit color

---
 src/term.c     | 28 ++++++++++++++++++++++------
 src/termchar.h | 10 ++++++----
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/term.c b/src/term.c
index b4f3dfc25e..8022ca95f1 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1945,7 +1945,10 @@ turn_on_face (struct frame *f, int face_id)
       ts = tty->standout_mode ? tty->TS_set_background : tty->TS_set_foreground;
       if (face_tty_specified_color (fg) && ts)
 	{
-          p = tparam (ts, NULL, 0, fg, 0, 0, 0);
+	  if (tty->TF_rgb_separate)
+	    p = tparam (ts, NULL, 0, fg >> 16, (fg >> 8) & 0xFF, fg & 0xFF, 0);
+	  else
+	    p = tparam (ts, NULL, 0, fg, 0, 0, 0);
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
@@ -1953,7 +1956,10 @@ turn_on_face (struct frame *f, int face_id)
       ts = tty->standout_mode ? tty->TS_set_foreground : tty->TS_set_background;
       if (face_tty_specified_color (bg) && ts)
 	{
-          p = tparam (ts, NULL, 0, bg, 0, 0, 0);
+	  if (tty->TF_rgb_separate)
+	    p = tparam (ts, NULL, 0, bg >> 16, (bg >> 8) & 0xFF, bg & 0xFF, 0);
+	  else
+	    p = tparam (ts, NULL, 0, bg, 0, 0, 0);
 	  OUTPUT (tty, p);
 	  xfree (p);
 	}
@@ -4133,10 +4139,10 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
 
 #ifdef TERMINFO
       {
-	const char *fg = tigetstr ("setf24");
-	const char *bg = tigetstr ("setb24");
-	/* Non-standard support for 24-bit colors. */
-	if (fg && bg
+	const char *fg;
+	const char *bg;
+	/* Our own non-standard support for 24-bit colors. */
+	if ((fg = tigetstr ("setf24")) && (bg = tigetstr ("setb24"))
 	    && fg != (char *) (intptr_t) -1
 	    && bg != (char *) (intptr_t) -1)
 	  {
@@ -4144,6 +4150,16 @@ init_tty (const char *name, const char *terminal_type, bool must_succeed)
 	    tty->TS_set_background = bg;
 	    tty->TN_max_colors = 16777216;
 	  }
+	/* Other non-standard support for 24-bit colors. */
+	else if ((fg = tigetstr ("setrgbf")) && (bg = tigetstr ("setrgbb"))
+	    && fg != (char *) (intptr_t) -1
+	    && bg != (char *) (intptr_t) -1)
+	  {
+	    tty->TS_set_foreground = fg;
+	    tty->TS_set_background = bg;
+	    tty->TN_max_colors = 16777216;
+	    tty->TF_rgb_separate = 1;
+	  }
 	/* Standard support for 24-bit colors.  */
 	else if (tigetflag ("RGB") > 0)
 	  {
diff --git a/src/termchar.h b/src/termchar.h
index 7ab9337fbe..d266fd16f9 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -154,10 +154,12 @@ #define EMACS_TERMCHAR_H
   /* "op" -- SVr4 set default pair to its original value.  */
   const char *TS_orig_pair;
 
-  /* "AF"/"AB" or "Sf"/"Sb"-- set ANSI or SVr4 foreground/background color.
-     1 param, the color index.  */
-  const char *TS_set_foreground;
-  const char *TS_set_background;
+  const char *TS_set_foreground; /* "AF"/"Sf"/"setrgbf" -- set foreground color. */
+  const char *TS_set_background; /* "AB"/"Sb"/"setrgbb" -- set background color. */
+  /* If set, TS_set_foreground and TS_set_background take 3 separate
+     params for R, G, and B values ("setrgbf"/"setrgbb" -- non-standard).
+     If unset, they take 1 param ("AF"/"AB" or "Sf"/"Sb" -- ANSI or SVr4r). */
+  int TF_rgb_separate;
 
   int TF_hazeltine;             /* termcap hz flag. */
   int TF_insmode_motion;        /* termcap mi flag: can move while in insert mode. */
-- 
2.33.1