Re: [PATCH] Support true colours via termcap interface

[Daniel Tameling <tamelingdaniel@xxxxxxxxx>]

On Thu, Feb 07, 2019 at 08:36:50AM +0100, Sebastian Gniazdowski wrote:
> 
> Could you restore the is_default_zle_highlight to its functioning state?
> 

Took me some time, but yes, I managed it.

In comparison to my last patch I made the following changes:

a) I had missed that for colours > 7 the termcap interface was used.
This is now unnecessary as the termcap attribute is returned whenever
it makes sense to use this interface. That the colours > 7 check was
still there broke true colours, so I removed that and now it's working
fine.

b) zle_highlight sequences are added to customize the true colour
escape sequences: fg_24bit_start_code, fg_24bit_delim and the
corresponding bg properties. For the end of the sequence, I used the
normal end_codes as they appear to be usually the same.

c) for the def case in set_colour_attribute, the true colour branch is
removed. I don't see any way that could be triggered, and having a
true colour version of "restore the default colour" doesn't really
make sense in my opinion.

d) move the is_default_zle_highlight logic to the place where the
zle_highlight array is checked. This has the advantage that one
doesn't need the string comparisons as the logic is implicit: if
zle_highlight is not NULL bypass termcap, else use it. This has also
the advantage that with "unset zle_highlight" you go back to using
termcap; to accomplish the same thing, you currently have to set the
codes you changed back to their default.

There is one last problem, I don't know how to address best: once you
use zle_highlight, you don't get the default sequences back if you
remove an entry from the array or unset it.

The X04 test passes as well. With TERM=xterm-direct only the two
nearcolor module tests fail, which only make sense for 256 colour
terminals. So everything looks good to me. I have tried to make the
test more terminal agnostic, but that is something for the next email.

Best regards,
Daniel


diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo
index c2b9f5430..7bbbba5cd 100644
--- a/Doc/Zsh/zle.yo
+++ b/Doc/Zsh/zle.yo
@@ -2682,6 +2682,12 @@ colour.
 item(tt(fg_end_code) (tt(m)))(
 The end of the escape sequence for the foreground colour.
 )
+item(tt(fg_24bit_start_code) (tt(\e[38;2;)))(
+The start of the escape sequence for the 24-bit foreground colour.
+)
+item(tt(fg_24bit_delim) (tt(;)))(
+The delimiter between the colours for the escape sequence for the 24-bit foreground colour.
+)
 item(tt(bg_start_code) (tt(\e[4)))(
 The start of the escape sequence for the background colour.
 See tt(fg_start_code) above.
@@ -2693,6 +2699,12 @@ background colour.
 item(tt(bg_end_code) (tt(m)))(
 The end of the escape sequence for the background colour.
 )
+item(tt(bg_24bit_start_code) (tt(\e[48;2;)))(
+The start of the escape sequence for the 24-bit background colour.
+)
+item(tt(bg_24bit_delim) (tt(;)))(
+The delimiter between the colours for the escape sequence for the 24-bit background colour.
+)
 enditem()
 
 The available types of highlighting are the following.  Note that
diff --git a/Src/prompt.c b/Src/prompt.c
index f2b3f161e..11442b69e 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -1621,7 +1621,6 @@ match_colour(const char **teststrp, int is_fg, int colour)
 {
     int shft, named = 0, tc;
     zattr on;
-
     if (is_fg) {
 	shft = TXT_ATTR_FG_COL_SHIFT;
 	on = TXTFGCOLOUR;
@@ -1652,6 +1651,14 @@ match_colour(const char **teststrp, int is_fg, int colour)
 	    colour = runhookdef(GETCOLORATTR, &color) - 1;
 	    if (colour == -1) { /* no hook function added, try true color (24-bit) */
 		colour = (((color.red << 8) + color.green) << 8) + color.blue;
+                /*
+                 * If we have a true colour termcap entry and colour > 7
+                 * use termcap; for colours 0-7 termcap usually emits the
+                 * standard ANSI sequences; we don't want that.
+                 */
+                if (tccolours == 0x1000000 && colour > 7) {
+                        on |= is_fg ? TXT_ATTR_FG_TERMCAP : TXT_ATTR_BG_TERMCAP;
+                }
 		return on | (is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT) |
 			(zattr)colour << shft;
 	    } else if (colour <= -2) {
@@ -1668,7 +1675,7 @@ match_colour(const char **teststrp, int is_fg, int colour)
 	}
 	else {
 	    colour = (int)zstrtol(*teststrp, (char **)teststrp, 10);
-	    if (colour < 0 || colour >= 256)
+	    if (colour < 0 || colour >= tccolours)
 		return TXT_ERROR;
 	}
     }
@@ -1692,6 +1699,16 @@ match_colour(const char **teststrp, int is_fg, int colour)
 	     */
 	    on |= is_fg ? TXT_ATTR_FG_TERMCAP :
 		TXT_ATTR_BG_TERMCAP;
+            /*
+             * If our terminal supports more than 256 colours it is
+             * most likely a true colour terminal (it's not always
+             * 256*256*256 colours: sometimes tccolours get truncated
+             * to the largest short, which is significantly smaller);
+             * if we don't use termcap, we want to emit true colour
+             * escape sequences for colour > 7.
+             */
+            if (tccolours > 256 && colour > 7)
+                    on |= is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT;
 	}
     }
     return on | (zattr)colour << shft;
@@ -1867,6 +1884,10 @@ output_highlight(zattr atr, char *buf)
 #define TC_COL_FG_END	"m"
 /* Code to reset foreground colour */
 #define TC_COL_FG_DEFAULT	"9"
+/* Start of true colour foreground escape sequence */
+#define TC_COL_FG_24BIT_START	"\033[38;2;"
+/* Delimiter for true colour foreground escape sequence */
+#define TC_COL_FG_24BIT_DELIM	";"
 
 /* Start of escape sequence for background colour */
 #define TC_COL_BG_START	"\033[4"
@@ -1874,11 +1895,17 @@ output_highlight(zattr atr, char *buf)
 #define TC_COL_BG_END	"m"
 /* Code to reset background colour */
 #define TC_COL_BG_DEFAULT	"9"
+/* Start of true colour background escape sequence  */
+#define TC_COL_BG_24BIT_START	"\033[48;2;"
+/* Delimiter for true colour background escape sequence */
+#define TC_COL_BG_24BIT_DELIM	";"
 
 struct colour_sequences {
     char *start;		/* Escape sequence start */
     char *end;			/* Escape sequence terminator */
     char *def;			/* Code to reset default colour */
+    char *start_24bit;		/* True colour escape sequence start */
+    char *delim_24bit;		/* Delimiter for true colour sequence */
 };
 static struct colour_sequences fg_bg_sequences[2];
 
@@ -1902,10 +1929,14 @@ set_default_colour_sequences(void)
     fg_bg_sequences[COL_SEQ_FG].start = ztrdup(TC_COL_FG_START);
     fg_bg_sequences[COL_SEQ_FG].end = ztrdup(TC_COL_FG_END);
     fg_bg_sequences[COL_SEQ_FG].def = ztrdup(TC_COL_FG_DEFAULT);
+    fg_bg_sequences[COL_SEQ_FG].start_24bit = ztrdup(TC_COL_FG_24BIT_START);
+    fg_bg_sequences[COL_SEQ_FG].delim_24bit = ztrdup(TC_COL_FG_24BIT_DELIM);
 
     fg_bg_sequences[COL_SEQ_BG].start = ztrdup(TC_COL_BG_START);
     fg_bg_sequences[COL_SEQ_BG].end = ztrdup(TC_COL_BG_END);
     fg_bg_sequences[COL_SEQ_BG].def = ztrdup(TC_COL_BG_DEFAULT);
+    fg_bg_sequences[COL_SEQ_BG].start_24bit = ztrdup(TC_COL_BG_24BIT_START);
+    fg_bg_sequences[COL_SEQ_BG].delim_24bit = ztrdup(TC_COL_BG_24BIT_DELIM);
 }
 
 static void
@@ -1919,6 +1950,12 @@ set_colour_code(char *str, char **var)
     *var = metafy(keyseq, len, META_DUP);
 }
 
+/*
+ * if non-zero try to use termcap to emit colors
+ * otherwise use fg_bg_sequences
+ */
+static int is_default_zle_highlight = 1;
+
 /* Allocate buffer for colour code composition */
 
 /**/
@@ -1926,7 +1963,7 @@ mod_export void
 allocate_colour_buffer(void)
 {
     char **atrs;
-    int lenfg, lenbg, len;
+    int lenfg, lenbg, len, len24bit;
 
     if (colseq_buf_allocs++)
 	return;
@@ -1940,33 +1977,59 @@ allocate_colour_buffer(void)
 		set_colour_code(*atrs + 16, &fg_bg_sequences[COL_SEQ_FG].def);
 	    } else if (strpfx("fg_end_code:", *atrs)) {
 		set_colour_code(*atrs + 12, &fg_bg_sequences[COL_SEQ_FG].end);
+	    } else if (strpfx("fg_24bit_start_code:", *atrs)) {
+		set_colour_code(*atrs + 20, &fg_bg_sequences[COL_SEQ_FG].start_24bit);
+	    } else if (strpfx("fg_24bit_delim:", *atrs)) {
+		set_colour_code(*atrs + 15, &fg_bg_sequences[COL_SEQ_FG].delim_24bit);
 	    } else if (strpfx("bg_start_code:", *atrs)) {
 		set_colour_code(*atrs + 14, &fg_bg_sequences[COL_SEQ_BG].start);
 	    } else if (strpfx("bg_default_code:", *atrs)) {
 		set_colour_code(*atrs + 16, &fg_bg_sequences[COL_SEQ_BG].def);
 	    } else if (strpfx("bg_end_code:", *atrs)) {
 		set_colour_code(*atrs + 12, &fg_bg_sequences[COL_SEQ_BG].end);
+	    } else if (strpfx("bg_24bit_start_code:", *atrs)) {
+		set_colour_code(*atrs + 20, &fg_bg_sequences[COL_SEQ_BG].start_24bit);
+	    } else if (strpfx("bg_24bit_delim:", *atrs)) {
+		set_colour_code(*atrs + 15, &fg_bg_sequences[COL_SEQ_BG].delim_24bit);
 	    }
 	}
-    }
+	is_default_zle_highlight = 0;
+    } else
+        is_default_zle_highlight = 1;
 
     lenfg = strlen(fg_bg_sequences[COL_SEQ_FG].def);
-    /* always need 1 character for non-default code */
-    if (lenfg < 1)
-	lenfg = 1;
+    /* make sure there is space for 3 digit colours */
+    if (lenfg < 3)
+	lenfg = 3;
     lenfg += strlen(fg_bg_sequences[COL_SEQ_FG].start) +
 	strlen(fg_bg_sequences[COL_SEQ_FG].end);
 
     lenbg = strlen(fg_bg_sequences[COL_SEQ_BG].def);
-    /* always need 1 character for non-default code */
-    if (lenbg < 1)
-	lenbg = 1;
+    /* make sure there is space for 3 digit colours */
+    if (lenbg < 3)
+	lenbg = 3;
     lenbg += strlen(fg_bg_sequences[COL_SEQ_BG].start) +
 	strlen(fg_bg_sequences[COL_SEQ_BG].end);
 
     len = lenfg > lenbg ? lenfg : lenbg;
-    /* add 1 for the null and 14 for truecolor */
-    colseq_buf = (char *)zalloc(len+15);
+
+    lenfg = strlen(fg_bg_sequences[COL_SEQ_FG].start_24bit) +
+            2 * strlen(fg_bg_sequences[COL_SEQ_FG].delim_24bit) +
+            strlen(fg_bg_sequences[COL_SEQ_FG].end);
+    /* add 9 for the colours */
+    lenfg += 9;
+
+    lenbg = strlen(fg_bg_sequences[COL_SEQ_BG].start_24bit) +
+            2 * strlen(fg_bg_sequences[COL_SEQ_BG].delim_24bit) +
+            strlen(fg_bg_sequences[COL_SEQ_BG].end);
+    /* add 9 for the colours */
+    lenbg += 9;
+
+    len24bit = lenfg > lenbg ? lenfg : lenbg;
+    len = len > len24bit ? len : len24bit;
+
+    /* add 1 for the null */
+    colseq_buf = (char *)zalloc(len+1);
 }
 
 /* Free the colour buffer previously allocated. */
@@ -2002,7 +2065,6 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
     char *ptr;
     int do_free, is_prompt = (flags & TSC_PROMPT) ? 1 : 0;
     int colour, tc, def, use_termcap, use_truecolor;
-    int is_default_zle_highlight = 1;
 
     if (fg_bg == COL_SEQ_FG) {
 	colour = txtchangeget(atr, TXT_ATTR_FG_COL);
@@ -2018,17 +2080,6 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
 	use_termcap = txtchangeisset(atr, TXT_ATTR_BG_TERMCAP);
     }
 
-    /* Test if current zle_highlight settings are customized, or
-     * the typical "standard" codes */
-    if (0 != strcmp(fg_bg_sequences[fg_bg].start, fg_bg == COL_SEQ_FG ? TC_COL_FG_START : TC_COL_BG_START) ||
-	/* the same in-fix for both FG and BG */
-	0 != strcmp(fg_bg_sequences[fg_bg].def, TC_COL_FG_DEFAULT) ||
-	/* the same suffix for both FG and BG */
-	0 != strcmp(fg_bg_sequences[fg_bg].end, TC_COL_FG_END))
-    {
-	is_default_zle_highlight = 0;
-    }
-
     /*
      * If we're not restoring the default, and either have a
      * colour value that is too large for ANSI, or have been told
@@ -2039,8 +2090,7 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
      * highlighting variables, so much of this shouldn't be
      * necessary at this point, but we might as well be safe.
      */
-    if (!def && !use_truecolor &&
-	(is_default_zle_highlight && (colour > 7 || use_termcap)))
+    if (!def && is_default_zle_highlight && use_termcap)
     {
 	/*
 	 * We can if it's available, and either we couldn't get
@@ -2083,31 +2133,27 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
      * or the typical true-color code: .start + 8;2;%d;%d;%d + .end
      * or the typical 256-color code: .start + 8;5;%d + .end
      */
-    if (use_truecolor)
-	strcpy(colseq_buf, fg_bg == COL_SEQ_FG ? TC_COL_FG_START : TC_COL_BG_START);
+    if (use_truecolor){
+	strcpy(colseq_buf, fg_bg_sequences[fg_bg].start_24bit);
+    }
     else
 	strcpy(colseq_buf, fg_bg_sequences[fg_bg].start);
 
     ptr = colseq_buf + strlen(colseq_buf);
     if (def) {
-	if (use_truecolor)
-	    strcpy(ptr, fg_bg == COL_SEQ_FG ? TC_COL_FG_DEFAULT : TC_COL_BG_DEFAULT);
-	else
-	    strcpy(ptr, fg_bg_sequences[fg_bg].def);
+	strcpy(ptr, fg_bg_sequences[fg_bg].def);
 	while (*ptr)
 	    ptr++;
     } else if (use_truecolor) {
-	ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
-		(colour >> 8) & 0xff, colour & 0xff);
+	ptr += sprintf(ptr, "%d%s%d%s%d", colour >> 16,
+                fg_bg_sequences[fg_bg].delim_24bit, (colour >> 8) & 0xff,
+                fg_bg_sequences[fg_bg].delim_24bit, colour & 0xff);
     } else if (colour > 7 && colour <= 255) {
 	ptr += sprintf(ptr, "%d", colour);
     } else
 	*ptr++ = colour + '0';
-    if (use_truecolor)
-	strcpy(ptr, fg_bg == COL_SEQ_FG ? TC_COL_FG_END : TC_COL_BG_END);
-    else
-	strcpy(ptr, fg_bg_sequences[fg_bg].end);
 
+    strcpy(ptr, fg_bg_sequences[fg_bg].end);
     if (is_prompt) {
 	if (!bv->dontcount) {
 	    addbufspc(1);
diff --git a/Test/X04zlehighlight.ztst b/Test/X04zlehighlight.ztst
index 000949698..c44202bd7 100644
--- a/Test/X04zlehighlight.ztst
+++ b/Test/X04zlehighlight.ztst
@@ -13,7 +13,7 @@
       zpty -d
       zpty zsh "${(q)ZTST_testdir}/../Src/zsh -fiV +Z"
       zpty -w zsh "module_path=( ${(j< >)${(@q-)module_path}} \$module_path )"
-      zpty -w zsh 'zle_highlight=( fg_start_code:"CDE|3" fg_end_code:"|" bg_start_code:"BCDE|4" bg_end_code:"|" )'
+      zpty -w zsh 'zle_highlight=( fg_start_code:"CDE|3" fg_24bit_start_code:"FGH|3|" fg_end_code:"|" bg_start_code:"BCDE|4|" bg_24bit_start_code:"FGH|4" bg_end_code:"|" )'
     }
     zpty_input() {
       zpty ${${(M)2:#nonl}:+-n} -w zsh "$1"
@@ -103,7 +103,7 @@
   zpty_line 1 p       # the line of interest, preserving escapes ("p")
   zpty_stop
 0:basic region_highlight with true-color (hex-triplets)
->0m27m24m38;2;4;8;16mtrueCDE|39|
+>0m27m24mFGH|3|4;8;16|trueCDE|39|
 
   zpty_start
   zpty_input 'zmodload zsh/nearcolor'
@@ -139,7 +139,7 @@
   zpty_line 1 p       # the line of interest, preserving escapes ("p")
   zpty_stop
 0:overlapping region_highlight with true-color
->0m27m24m38;2;0;204;0mt38;2;204;0;0mrCDE|39|38;2;0;204;0mueCDE|39|
+>0m27m24mFGH|3|0;204;0|tFGH|3|204;0;0|rCDE|39|FGH|3|0;204;0|ueCDE|39|
 
   zpty_start
   zpty_input 'zmodload zsh/nearcolor'