[PATCH] Make 256 color codes be based on zle_highlight array, not on termcap

[PATCH] Make 256 color codes be based on zle_highlight array, not on termcap

[Sebastian Gniazdowski]

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

Hello,
zle_highlight array has fields like `fg_start_code' & `fg_end_code'.
The first is by default $'\x1b[', the second - 'm'.

ANSI colors and True-Colors are using those fields to construct and
emit the final, complete escape (by default) code to the terminal.

Yet the 248 colors are ignoring zle_highlight and are equipped with
terminal code by use of termcap. I think the following line (prompt.c:
~2044) does the conversion:

                tputs(tgoto(tcstr[tc], colour, colour), 1, putshout);

The patch makes the 256 colors to also use zle_highlight. This is the
main addition of the patch (also included the TrueColor case, to show
that this is done exactly this way):

    } else if (use_truecolor) {
        ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
                (colour >> 8) & 0xff, colour & 0xff);
+    } else if (colour > 7 && colour <= 255) {
+        ptr += sprintf(ptr, "8;5;%d", colour);

Is the patch acceptable? I imagine someone might suspect there exists
a terminal that uses escape codes for first 8 colors, but some other
kind of codes, collected in termcap, for remaining 248 colors. Can
this be considered impossible to accept the patch?

BTW. I've made X04 zle tests more rock-solid, because the debug prints
I added to track the 256-color/zle_highlight issue were slowing down
Zle, and a rare phenomenon (not occurred from the time I've simplified
Zle, disabled the echoing of input text) became very often: Ctrl-D was
appearing too quickly after Ctrl-A. A sleep of 333 ms made the problem
go away, and I think the issue is solved, and Zle tests are quite rock
solid now.
-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

[-- Attachment #2: 256-colors-to-use-zle_highlight.diff.txt --]
[-- Type: text/plain, Size: 2540 bytes --]

commit 5dda212b3be3f1f12ad31e3d6543d367f987ee55
Author: Sebastian Gniazdowski <sgniazdowski@gmail.com>
Date:   Sat Dec 8 22:42:49 2018 +0100

    Instead of using termcap, 256-colours are based on zle_highlight entries
    
    Following code snippet performs the conversion of colour number to the
    code to be emitted to terminal (the example shows foreground color
    handling):
        ...
        strcpy(colseq_buf, fg_bg_sequences[fg_bg].start);
        ...
        } else if (colour > 7 && colour <= 255) {
            ptr += sprintf(ptr, "8;5;%d", colour);
        ...

diff --git a/Src/prompt.c b/Src/prompt.c
index 568bfc2a9..4a90f5fcf 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -1764,7 +1764,9 @@ output_colour(int colour, int fg_bg, int use_tc, int truecol, char *buf)
 	/* length of hex triplet always 7, don't need sprintf to count */
 	atrlen += buf ? sprintf(ptr, "#%02x%02x%02x", colour >> 16,
 		(colour >> 8) & 0xff, colour & 0xff) : 7;
-    /* colour should only be > 7 if using termcap but let's be safe */
+    /* colour should only be > 7 if using termcap but let's be safe
+     * update: currently other places in code don't always imply
+     * that colour > 7 => using-termcap */
     } else if (use_tc || colour > 7) {
 	char digbuf[DIGBUFSIZE];
 	sprintf(digbuf, "%d", colour);
@@ -2020,7 +2022,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 && (colour > 7 || use_termcap)) {
+    if (!def && !use_truecolor && (colour > 255 && use_termcap)) {
 	/*
 	 * We can if it's available, and either we couldn't get
 	 * the maximum number of colours, or the colour is in range.
@@ -2046,9 +2048,10 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
 	}
 	/*
 	 * Nope, that didn't work.
-	 * If 0 to 7, assume standard ANSI works, otherwise it won't.
+	 * If 0 to 7, assume standard ANSI works, if 8 to 255, assume
+         * standard 256-color escapes works, otherwise it won't.
 	 */
-	if (colour > 7)
+	if (colour > 255)
 	    return;
     }
 
@@ -2067,6 +2070,8 @@ set_colour_attribute(zattr atr, int fg_bg, int flags)
     } else if (use_truecolor) {
 	ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
 		(colour >> 8) & 0xff, colour & 0xff);
+    } else if (colour > 7 && colour <= 255) {
+        ptr += sprintf(ptr, "8;5;%d", colour);
     } else
 	*ptr++ = colour + '0';
     strcpy(ptr, fg_bg_sequences[fg_bg].end);

Re: [PATCH] Make 256 color codes be based on zle_highlight array, not on termcap

[Daniel Shahaf]

Sebastian Gniazdowski wrote on Sat, Dec 08, 2018 at 23:04:33 +0100:
> Is the patch acceptable? I imagine someone might suspect there exists
> a terminal that uses escape codes for first 8 colors, but some other
> kind of codes, collected in termcap, for remaining 248 colors. Can
> this be considered impossible to accept the patch?

Might someone out there have a terminal whereunder the sequence ESC 8 ; 5 ; <number>
doesn't change the foreground color, but does something else?

> BTW. I've made X04 zle tests more rock-solid, because the debug prints
> I added to track the 256-color/zle_highlight issue were slowing down
> Zle, and a rare phenomenon (not occurred from the time I've simplified
> Zle, disabled the echoing of input text) became very often: Ctrl-D was
> appearing too quickly after Ctrl-A. A sleep of 333 ms made the problem
> go away, and I think the issue is solved, and Zle tests are quite rock
> solid now.

That's great.

> commit 5dda212b3be3f1f12ad31e3d6543d367f987ee55
> Author: Sebastian Gniazdowski <sgniazdowski@gmail.com>
> Date:   Sat Dec 8 22:42:49 2018 +0100
> 
>     Instead of using termcap, 256-colours are based on zle_highlight entries
>     
>     Following code snippet performs the conversion of colour number to the
>     code to be emitted to terminal (the example shows foreground color
>     handling):
>         ...
>         strcpy(colseq_buf, fg_bg_sequences[fg_bg].start);
>         ...
>         } else if (colour > 7 && colour <= 255) {
>             ptr += sprintf(ptr, "8;5;%d", colour);
>         ...

For future reference, it's better to post `git format-patch` output
than `git show` output because the former can be applied easily (with
git-am(1)) but the latter can't.

Cheers,

Daniel

Re: [PATCH] Make 256 color codes be based on zle_highlight array, not on termcap

[Sebastian Gniazdowski]

On Sun, 9 Dec 2018 at 20:45, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Sebastian Gniazdowski wrote on Sat, Dec 08, 2018 at 23:04:33 +0100:
> > Is the patch acceptable? I imagine someone might suspect there exists
> > a terminal that uses escape codes for first 8 colors, but some other
> > kind of codes, collected in termcap, for remaining 248 colors. Can
> > this be considered impossible to accept the patch?
>
> Might someone out there have a terminal whereunder the sequence ESC 8 ; 5 ; <number>
> doesn't change the foreground color, but does something else?

I've addressed such possibility in 43888 (i.e. in thread: "highlight
test cases (was Re: [BUG?] If true-color..."). In the proposal,
termcap would be used only when zle_highlight is not customized. This
way one will be able to made an actual use of zle_highlight, write
tests, do debugging, etc. and user will be still covered in case of
uncommon-terminal.

> > commit 5dda212b3be3f1f12ad31e3d6543d367f987ee55
> > Author: Sebastian Gniazdowski <sgniazdowski@gmail.com>
> > Date:   Sat Dec 8 22:42:49 2018 +0100
> >
> >     Instead of using termcap, 256-colours are based on zle_highlight entries
> >
> >     Following code snippet performs the conversion of colour number to the
> >     code to be emitted to terminal (the example shows foreground color
> >     handling):
> >         ...
> >         strcpy(colseq_buf, fg_bg_sequences[fg_bg].start);
> >         ...
> >         } else if (colour > 7 && colour <= 255) {
> >             ptr += sprintf(ptr, "8;5;%d", colour);
> >         ...
>
> For future reference, it's better to post `git format-patch` output
> than `git show` output because the former can be applied easily (with
> git-am(1)) but the latter can't.

Thanks!
-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org