zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] Support true colours via termcap interface
@ 2019-02-03 21:57 Daniel Tameling
  2019-02-04  2:15 ` Sebastian Gniazdowski
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Tameling @ 2019-02-03 21:57 UTC (permalink / raw)
  To: zsh-workers

Dear all,

ncurses recently added support for 24 bit colour terminals. It kind of
works out of the box with zsh, except for the hardcoded 256 colour
limit in match_colour. I think previously you could get away with
using colours 0-7 when tccolours is less than 8, which now would emit
the default text escape sequence. I don't think this change should
cause any trouble, but if the old behaviour needs to be preserved, it
can be done by changing the corresponding line below to
if (colour < 0 || (colour > 7 && colour >= tccolours))

I also let the termcap interface handle the recently implemented true
colours, which required only a minimal change as ncurses' terminfo
entries use the same encoding as Oliver's implementation. For colours
0-7, however, one has to bypass the termcap interface as the terminfo
entries specify for that range the standard ansi colours.

I didn't touch the hardcoded escape sequence for true colours because,
although there is some dispute on how the escape sequence should look
like, it seems that this sequence is recognized by all terminals with
true colours support. Please note that the disagreement revolves
around whether semicolons or colons should be used as separators, so
if the situation changes, the current approach of being able to
customize only the start and the end of the escape sequence would need
to be extended to let users specify the separator.

In summary, the patch enables the following:

$ TERM=xterm-direct print -P %F{'#ffcc33'} | cat -v
^[[38:2::255:204:51m

$ TERM=xterm-direct print -P %F{16763955} | cat -v
^[[38:2::255:204:51m

which is what tput delivers:

$ TERM=xterm-direct tput setaf 16763955 | cat -v
^[[38:2::255:204:51m

-- 
Best regards,
Daniel


diff --git a/Src/prompt.c b/Src/prompt.c
index f2b3f161e..589ee7664 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -1652,8 +1652,10 @@ 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;
-		return on | (is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT) |
-			(zattr)colour << shft;
+                if (tccolours != 0x1000000 || colour < 8) {
+                        return on | (is_fg ? TXT_ATTR_FG_24BIT :
+                                     TXT_ATTR_BG_24BIT) | (zattr)colour << shft;
+                }
 	    } else if (colour <= -2) {
 		return TXT_ERROR;
 	    }
@@ -1668,7 +1670,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;
 	}
     }


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-03 21:57 [PATCH] Support true colours via termcap interface Daniel Tameling
@ 2019-02-04  2:15 ` Sebastian Gniazdowski
  2019-02-04  6:19   ` Daniel Tameling
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Gniazdowski @ 2019-02-04  2:15 UTC (permalink / raw)
  To: zsh-workers

> @@ -1652,8 +1652,10 @@ match_colour(const char **teststrp, int is_fg, int colour)
On Sun, 3 Feb 2019 at 22:57, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
>
> Dear all,
>
> ncurses recently added support for 24 bit colour terminals. It kind of
> works out of the box with zsh, except for the hardcoded 256 colour
> limit in match_colour. olor.red << 8) + color.green) << 8) + color.blue;
(...)
>
> -               return on | (is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT) |
> -                       (zattr)colour << shft;
> +                if (tccolours != 0x1000000 || colour < 8) {
> +                        return on | (is_fg ? TXT_ATTR_FG_24BIT :
> +                                     TXT_ATTR_BG_24BIT) | (zattr)colour << shft;
> +                }
>             } else if (colour <= -2) {
>                 return TXT_ERROR;
>             }

I wonder what this change causes. Because to use the termcap for 24
bit sequences, noe would have to lessen the following conditions in
set_colour_attribute():

Line 2040:     if (!def && !use_truecolor &&
Line 2041:        (is_default_zle_highlight && (colour > 7 || use_termcap)))
Line 2042:    {

I.e. remove  or in other way lessen the use_truecolor condition.
Without this the true color output will be bypassing termcap.

Currently, the code  `print -P %F{16763955} ' DOESN'T work, so your
change does something that makes it working, however the first code
example:

print -P %F{'#ffcc33'} test

Works with current code (ie. without the patch).
-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-04  2:15 ` Sebastian Gniazdowski
@ 2019-02-04  6:19   ` Daniel Tameling
  2019-02-04  8:11     ` Sebastian Gniazdowski
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Tameling @ 2019-02-04  6:19 UTC (permalink / raw)
  To: zsh-workers

On Mon, Feb 04, 2019 at 03:15:13AM +0100, Sebastian Gniazdowski wrote:
> > @@ -1652,8 +1652,10 @@ match_colour(const char **teststrp, int is_fg, int colour)
> On Sun, 3 Feb 2019 at 22:57, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
> >
> > Dear all,
> >
> > ncurses recently added support for 24 bit colour terminals. It kind of
> > works out of the box with zsh, except for the hardcoded 256 colour
> > limit in match_colour. olor.red << 8) + color.green) << 8) + color.blue;
> (...)
> >
> > -               return on | (is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT) |
> > -                       (zattr)colour << shft;
> > +                if (tccolours != 0x1000000 || colour < 8) {
> > +                        return on | (is_fg ? TXT_ATTR_FG_24BIT :
> > +                                     TXT_ATTR_BG_24BIT) | (zattr)colour << shft;
> > +                }
> >             } else if (colour <= -2) {
> >                 return TXT_ERROR;
> >             }
> 
> I wonder what this change causes. Because to use the termcap for 24
> bit sequences, noe would have to lessen the following conditions in
> set_colour_attribute():
> 
> Line 2040:     if (!def && !use_truecolor &&
> Line 2041:        (is_default_zle_highlight && (colour > 7 || use_termcap)))
> Line 2042:    {
> 
> I.e. remove  or in other way lessen the use_truecolor condition.
> Without this the true color output will be bypassing termcap.

That is not necessary. use_truecolor is false and use_termcap is true,
because TXT_ATTR_FG_24BIT isn't returned by match_colour but
TXT_ATTR_FG_TERMCAP.

Btw. I did that deliberately. I didn't want to make the if even more
complicated and I wanted to use the same code path as for the
%F{16763955} syntax. Making a small change to match_colour felt nicer
than to hack around in set_colour_attribute.

> 
> Currently, the code  `print -P %F{16763955} ' DOESN'T work, so your
> change does something that makes it working

It doesn't work because match_colour currently returns TXT_ERROR for
this sort of syntax if colour >= 256.

> , however the first code
> example:
> 
> print -P %F{'#ffcc33'} test
> 
> Works with current code (ie. without the patch).

Well, it works differently:

Before the patch, the hardcoded escape sequence is used: 
$ TERM=xterm-direct print -P %F{'#ffcc00'} | cat -v
^[[38;2;255;204;0m

Afterwards the terminfo entry is used:
$ TERM=xterm-direct print -P %F{'#ffcc33'} | cat -v
^[[38:2::255:204:51m

The former works because xterm understands both escape sequences, but
that might not be the case for all terminals out there. It seems that
right now all terminals that deviate from the former syntax still
interpret it correctly. But there is no guarantee that the situation
will stay this way. In general, it should be more future proof to use
the termcap entry if it is available.

-- 
Best regards,
Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-04  6:19   ` Daniel Tameling
@ 2019-02-04  8:11     ` Sebastian Gniazdowski
  2019-02-04 21:21       ` Daniel Tameling
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Gniazdowski @ 2019-02-04  8:11 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, 4 Feb 2019 at 07:20, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
> > I.e. remove  or in other way lessen the use_truecolor condition.
> > Without this the true color output will be bypassing termcap.
>
> That is not necessary. use_truecolor is false and use_termcap is true,
> because TXT_ATTR_FG_24BIT isn't returned by match_colour but
> TXT_ATTR_FG_TERMCAP.
>
> Btw. I did that deliberately. I didn't want to make the if even more
> complicated and I wanted to use the same code path as for the
> %F{16763955} syntax. Making a small change to match_colour felt nicer
> than to hack around in set_colour_attribute.

Ahso,,ok, but I don't see the *_TERMCAP code being returned here,
could you explain?

> > -               return on | (is_fg ? TXT_ATTR_FG_24BIT : TXT_ATTR_BG_24BIT) |
> > -                       (zattr)colour << shft;
> > +                if (tccolours != 0x1000000 || colour < 8) {
> > +                        return on | (is_fg ? TXT_ATTR_FG_24BIT :
> > +                                     TXT_ATTR_BG_24BIT) | (zattr)colour << shft;
> > +                }

> >
> > Currently, the code  `print -P %F{16763955} ' DOESN'T work, so your
> > change does something that makes it working
>
> It doesn't work because match_colour currently returns TXT_ERROR for
> this sort of syntax if colour >= 256.

Ahso, ok.

> > , however the first code
> > example:
> >
> > print -P %F{'#ffcc33'} test
> >
> > Works with current code (ie. without the patch).
>
> Well, it works differently:
>
> Before the patch, the hardcoded escape sequence is used:
> $ TERM=xterm-direct print -P %F{'#ffcc00'} | cat -v
> ^[[38;2;255;204;0m
>
> Afterwards the terminfo entry is used:
> $ TERM=xterm-direct print -P %F{'#ffcc33'} | cat -v
> ^[[38:2::255:204:51m
>
> The former works because xterm understands both escape sequences, but

Ahso, OK. Let me just point the difference: the second code has two
clons after "38:2".

BTW., I recomend the following lecture (created by an iTerm hacker) to
know more about the termcap-codes deviations:

https://terminalguide.netlify.com/attr/fgcol256/
https://terminalguide.netlify.com/attr/fgdirectcolor/


That said: does the new X04 test pass with your patch?

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-04  8:11     ` Sebastian Gniazdowski
@ 2019-02-04 21:21       ` Daniel Tameling
  2019-02-07  7:36         ` Sebastian Gniazdowski
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Tameling @ 2019-02-04 21:21 UTC (permalink / raw)
  To: zsh-workers

On Mon, Feb 04, 2019 at 09:11:04AM +0100, Sebastian Gniazdowski wrote:
> On Mon, 4 Feb 2019 at 07:20, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
> > Btw. I did that deliberately. I didn't want to make the if even more
> > complicated and I wanted to use the same code path as for the
> > %F{16763955} syntax. Making a small change to match_colour felt nicer
> > than to hack around in set_colour_attribute.
> 
> Ahso,,ok, but I don't see the *_TERMCAP code being returned here,
> could you explain?

The code doesn't return from the function but continues and enters
if (!named && tccan(tc)) {
and there the else branch where
on |= is_fg ? TXT_ATTR_FG_TERMCAP : TXT_ATTR_BG_TERMCAP;
is executed. This is the same as for the %F{16763955} syntax.

> > Before the patch, the hardcoded escape sequence is used:
> > $ TERM=xterm-direct print -P %F{'#ffcc00'} | cat -v
> > ^[[38;2;255;204;0m
> >
> > Afterwards the terminfo entry is used:
> > $ TERM=xterm-direct print -P %F{'#ffcc33'} | cat -v
> > ^[[38:2::255:204:51m
> >
> > The former works because xterm understands both escape sequences, but
> 
> Ahso, OK. Let me just point the difference: the second code has two
> clons after "38:2".
> 

There are currently three definitions floating around (and also
present in ncurses' terminfo):
\e38;2;red;green;blue;m
\e38:2:red:green:blue:m
\e38:2::red:green:blue:m
The comments in ncurses terminfo briefly describe the history of these
sequences [1], which boils down to the following: The first one is a
natural extension of xterm's 256 colour sequences. The last one is
specified in a document from the nineties that tried to standardize
terminals. The second one stems from overlooking the a colour space
identifier in the document. If you want to know more about this you
should look at [2] and the comments to [3]. There is currently an
argument, whether the first or the last sequence should be used, but
it seems that at the moment all terminal support the syntax with
semicolons and some support additionally the one with colons.

[1]: https://invisible-island.net/ncurses/terminfo.src.html#tic-xterm_direct2
[2]: https://bugs.kde.org/show_bug.cgi?id=107487
[3]: https://gist.github.com/XVilka/8346728

> 
> That said: does the new X04 test pass with your patch?
> 

Yes, it does. But I noticed that's misleading as the test sets TERM to
xterm-256color. If I set it to xterm-direct the test fails even
without the patch
--- /tmp/zsh.ztst.75451/ztst.out        2019-02-04 20:11:21.000000000
+0100
+++ /tmp/zsh.ztst.75451/ztst.tout       2019-02-04 20:11:21.000000000
+0100
@@ -1 +1 @@
-0m27m24mCDE|3232|trueCDE|39|
+0m27m24mtrue
Test ./X04zlehighlight.ztst failed: output differs from expected as
shown above for:
zpty_start
zpty_input 'zmodload zsh/nearcolor'
zpty_input 'rh_widget() { BUFFER="true"; region_highlight+=( "0 4 fg=#040810" ); }'
zpty_input 'zle -N rh_widget'
zpty_input 'bindkey "\C-a" rh_widget'
zpty_enable_zle
zpty_input $'\C-a'  # emits newline, which executes BUFFER="true" command
zpty_line 1 p       # the line of interest, preserving escapes ("p")
zpty_stop
Was testing: basic region_highlight with near-color (hex-triplets at input)

But I also noticed that my patch breaks the is_default_zle_highlight =
0 code. It doesn't set use_truecolor for the #ffcc00 syntax. But that
is not the only problem: with a true colour terminal, one expects
colours beyond 255 to work, e.g. via fg=273. In that case one also has
to use the true colour escape sequence. Then also colours in the range
from 8 to 255 should use this sequence. But colours from 0 to 7 should
probably still be the standard ansi sequence, like ncurses does it.

One could achieve that by modifying the if(use_truecolor) checks when
the escape sequence is constructed manually. Alternatively, the patch
below should accomplish the same. Now zlehighlight.ztst fails at the
same point as without the patch for xterm-direct. It seems that this
is because the non-termcap code emits only the standard true colour
escape sequence, which cannot be modified like the other ones.

-- 
Best regards,
Daniel


diff --git a/Src/prompt.c b/Src/prompt.c
index f2b3f161e..bef53b148 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -1652,6 +1652,13 @@ 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;
@@ -2039,8 +2056,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 && (colour > 7 || use_termcap)))
     {
 	/*
 	 * We can if it's available, and either we couldn't get

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-04 21:21       ` Daniel Tameling
@ 2019-02-07  7:36         ` Sebastian Gniazdowski
  2019-02-07 20:32           ` Daniel Tameling
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Gniazdowski @ 2019-02-07  7:36 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, 4 Feb 2019 at 22:22, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
>
> On Mon, Feb 04, 2019 at 09:11:04AM +0100, Sebastian Gniazdowski wrote:
> > On Mon, 4 Feb 2019 at 07:20, Daniel Tameling <tamelingdaniel@gmail.com> wrote:
> But I also noticed that my patch breaks the is_default_zle_highlight =
> 0 code. It doesn't set use_truecolor for the #ffcc00 syntax.

Could you restore the is_default_zle_highlight to its functioning state?

> But that
> is not the only problem: with a true colour terminal, one expects
> colours beyond 255 to work, e.g. via fg=273. In that case one also has
> to use the true colour escape sequence. Then also colours in the range
> from 8 to 255 should use this sequence. But colours from 0 to 7 should
> probably still be the standard ansi sequence, like ncurses does it.
>
> One could achieve that by modifying the if(use_truecolor) checks when
> the escape sequence is constructed manually. Alternatively, the patch
> below should accomplish the same. Now zlehighlight.ztst fails at the
> same point as without the patch for xterm-direct. It seems that this
> is because the non-termcap code emits only the standard true colour
> escape sequence, which cannot be modified like the other ones.
>
> --
> Best regards,
> Daniel
>
>
> diff --git a/Src/prompt.c b/Src/prompt.c
> index f2b3f161e..bef53b148 100644
> --- a/Src/prompt.c
> +++ b/Src/prompt.c
> @@ -1652,6 +1652,13 @@ 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;
> @@ -2039,8 +2056,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 && (colour > 7 || use_termcap)))
>      {
>         /*
>          * We can if it's available, and either we couldn't get



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support true colours via termcap interface
  2019-02-07  7:36         ` Sebastian Gniazdowski
@ 2019-02-07 20:32           ` Daniel Tameling
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Tameling @ 2019-02-07 20:32 UTC (permalink / raw)
  To: zsh-workers

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'

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-02-07 20:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-03 21:57 [PATCH] Support true colours via termcap interface Daniel Tameling
2019-02-04  2:15 ` Sebastian Gniazdowski
2019-02-04  6:19   ` Daniel Tameling
2019-02-04  8:11     ` Sebastian Gniazdowski
2019-02-04 21:21       ` Daniel Tameling
2019-02-07  7:36         ` Sebastian Gniazdowski
2019-02-07 20:32           ` Daniel Tameling

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).