PATCH: broader support for highlight groups

PATCH: broader support for highlight groups

[Oliver Kiddle]

This broadens the support for %H to completion explanation strings
(with and without complist) and WATCHFMT, both of which already support
%B/%S/%U/%F/%K. This also affects the list-prompt style as a
side-effect.

Completion for prompt strings also gains support for %H.

The return value from parsehighlight() has been changed to go one
past the end character because that's now convenient for more of the
callers than leaving it on the end character. The one change of int to
size_t in watch.c is to silence a (-Wextra) compiler warning.

We still lack a way to reset attributes. Rather than %r, which does
have tenuous conflicts with completion explanations and some obscure
uses of zformat, I'm tending towards special-casing %H{reset} and/or
%H{none}. And perhaps disabling the feature of allowing left-over
attributes in the prompt to bleed over into the user's input where those
attributes came from a highlight group. That feature only exists for
backward-compatibility and anyone using a new feature like highlight
groups can also set the default key in zle_highlight. Any opinions?

Oliver

diff --git a/Completion/Zsh/Type/_ps1234 b/Completion/Zsh/Type/_ps1234
index 07dea5905..e4391dc00 100644
--- a/Completion/Zsh/Type/_ps1234
+++ b/Completion/Zsh/Type/_ps1234
@@ -1,17 +1,18 @@
 #compdef -value-,PROMPT,-default- -value-,PROMPT2,-default- -value-,PROMPT3,-default- -value-,PROMPT4,-default- -value-,RPROMPT,-default- -value-,RPROMPT2,-default- -value-,PS1,-default- -value-,PS2,-default- -value-,PS3,-default- -value-,PS4,-default- -value-,RPS1,-default- -value-,RPS2,-default- -value-,SPROMPT,-default- -value-,PROMPT_EOL_MARK,-default-
 
-local -a specs ccol
-local expl grp cols bs suf pre changed=1 ret=1
+local -a specs ccol suf
+local expl grp cols bs pre changed=1 ret=1
 local -A ansi
 
 [[ -z $compstate[quote] ]] && bs='\'
+suf=( -S '' )
 
 # first strip off any complete prompt specifications leaving only the
 # current, incomplete, one
 while (( changed )); do
   changed=0
-  compset -P '%[DFK](\\|){[^}]#}' && changed=1 # formats with arg: %x{...}
-  compset -P '%[0-9-\\]#[^DFK(0-9-<>\\\[]' && changed=1 # normal formats
+  compset -P '%[DFHK](\\|){[^}]#}' && changed=1 # formats with arg: %x{...}
+  compset -P '%[0-9-\\]#[^DFHK(0-9-<>\\\[]' && changed=1 # normal formats
   compset -P '%[0-9-\\]#(<[^<]#<|>[^>]#>|\[[^\]]#\])' && changed=1 # truncations
   compset -P '%[0-9-\\]#(\\|)\([0-9-]#[^0-9]?|[^%]' && changed=1 # start of ternary
   compset -P '[^%]##' && changed=1 # sundry other characters
@@ -41,15 +42,15 @@ if compset -P '%[FK]'; then
   grp="$expl[expl[(i)-J]+1]"
   print -v ccol -f "($grp)=%s=%s" ${(kv)ansi}
   _comp_colors+=( $ccol )
-  compadd "$expl[@]" $suf $pre -k ansi && ret=0
-  if (( $#suf )) && compset -P "(<->|%v)"; then
+  compadd "$expl[@]" "$suf[@]" $pre -k ansi && ret=0
+  if [[ $ISUFFIX != (\\|)}* ]] && compset -P "(<->|%v)"; then
     _wanted ansi-colors expl 'closing brace' compadd -S '' \} && ret=0
   elif (( $+terminfo[colors] )); then
     (( cols = $terminfo[colors] - 1 ))
     (( cols = cols > 255 ? 255 : cols ))
     _description -V terminal-colors expl 'terminal color'
     grp="$expl[expl[(i)-J]+1]"
-    compadd "$expl[@]" $suf $pre {0..$cols}
+    compadd "$expl[@]" "$suf[@]" $pre {0..$cols}
     for c in {0..$cols}; do
       _comp_colors+=( "($grp)=${c}=${${${(%):-%F{$c\}}#?\[}%m}" )
     done
@@ -93,11 +94,14 @@ elif compset -P '%[0-9-\\]#(\\|)\([0-9-]#'; then
     'w:day of week (Sunday = 0)'
   )
   [[ $IPREFIX != *- ]] && _describe -t ternary-prompt-expressions \
-      'ternary prompt format test character' specs $suf && ret=0
+      'ternary prompt format test character' specs "$suf[@]" && ret=0
   _message -e numbers number
 elif compset -P '%D(\\|){'; then
   compset -S '(\\|)}*'
   _date_formats zsh && ret=0
+elif compset -P '%H(\\|){'; then
+  compset -S '(\\|)}*' || suf=( -S "$bs}" )
+  _wanted highlight-groups expl 'highlight group' compadd "$suf[@]" -k .zle.hlgroups && ret=0
 elif [[ -prefix '%' ]] ||
       ! zstyle -t ":completion:${curcontext}:prompt-format-specifiers" prefix-needed
 then
@@ -152,6 +156,7 @@ then
       'B:start bold'
       'b:stop bold'
       'E:clear to end of line'
+      'H{:use highlight group'
       'U:start underline'
       'u:stop underline'
       'S:start standout'
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 3f708eb5a..f75298a1b 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -2023,8 +2023,8 @@ position shown as a percentage of the total length otherwise.  In each
 case the form with the uppercase letter will be replaced by a string of fixed
 width, padded to the  right with spaces, while the lowercase form will
 be replaced by a variable width string.  As in other prompt strings, the
-escape sequences `tt(%S)', `tt(%s)', `tt(%B)', `tt(%b)', `tt(%U)',
-`tt(%u)' for entering and leaving the display modes
+escape sequence `tt(%H)` along with `tt(%S)', `tt(%s)', `tt(%B)', `tt(%b)',
+`tt(%U)', `tt(%u)' for entering and leaving the display modes
 standout, bold and underline, and `tt(%F)', `tt(%f)', `tt(%K)', `tt(%k)' for
 changing the foreground background colour, are also available, as is the form
 `tt(%{)...tt(%})' for enclosing escape sequences which display with zero
diff --git a/Doc/Zsh/compwid.yo b/Doc/Zsh/compwid.yo
index 9461ace17..b0c9b0a5f 100644
--- a/Doc/Zsh/compwid.yo
+++ b/Doc/Zsh/compwid.yo
@@ -597,7 +597,8 @@ ifnzman((see noderef(Prompt Expansion)))\
 ifzman(as described in the section EXPANSION OF PROMPT SEQUENCES in
 zmanref(zshmisc)):
 `tt(%B)', `tt(%S)', `tt(%U)', `tt(%F)', `tt(%K)' and their lower case
-counterparts, as well as `tt(%{)...tt(%})'.  `tt(%F)', `tt(%K)' and
+counterparts, as well as `tt(%H)' and `tt(%{)...tt(%})'.  `tt(%F)',
+`tt(%K)', `tt(%H)' and
 `tt(%{)...tt(%})' take arguments in the same form as prompt
 expansion.  (Note that the sequence `tt(%G)' is not available; an
 argument to `tt(%{)' should be used instead.)  The sequence `tt(%%)'
diff --git a/Src/Modules/watch.c b/Src/Modules/watch.c
index 97d4fa608..ba17cf940 100644
--- a/Src/Modules/watch.c
+++ b/Src/Modules/watch.c
@@ -373,6 +373,13 @@ watchlog2(int inout, WATCH_STRUCT_UTMP *u, char *fmt, int prnt, int fini)
 		case 'f':
 		    tunsetattrs(TXTFGCOLOUR);
 		    break;
+		case 'H':
+		    if (*fmt == '{') {
+			fmt = parsehighlight(fmt + 1, '}', &atr);
+			if (atr && atr != TXT_ERROR)
+			    treplaceattrs(atr);
+		    }
+		    break;
 		case 'K':
 		    if (*fmt == '{') {
 			fmt++;
@@ -428,7 +435,7 @@ watchlog_match(char *teststr, char *actual, size_t buflen)
     int ret = 0;
     Patprog pprog;
     char *str = dupstring(teststr);
-    int len = strnlen(actual, buflen);
+    size_t len = strnlen(actual, buflen);
     char *user = metafy(actual, len,
 	    len == buflen ? META_HEAPDUP : META_USEHEAP);
 
diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
index 9cb89a60d..5619160a9 100644
--- a/Src/Zle/complist.c
+++ b/Src/Zle/complist.c
@@ -1181,6 +1181,13 @@ compprintfmt(char *fmt, int n, int dopr, int doesc, int ml, int *stop)
 		    if (dopr)
 			tunsetattrs(TXTBGCOLOUR);
 		    break;
+		case ZWC('H'):
+		    if (*p == '{') {
+			p = parsehighlight(p + 1, '}', &atr);
+			if (atr != TXT_ERROR && dopr)
+			    treplaceattrs(atr);
+		    }
+		    break;
 		case ZWC('{'):
 		    if (arg)
 			cc += arg;
diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index 225ce8c74..aa3c71bc2 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -2501,6 +2501,14 @@ printfmt(char *fmt, int n, int dopr, int doesc)
 		case 'k':
 		    tunsetattrs(TXTBGCOLOUR);
 		    break;
+		case 'H':
+		    if (p[1] == '{') {
+			p = parsehighlight(p + 2, '}', &atr);
+			--p;
+			if (atr != TXT_ERROR)
+			    treplaceattrs(atr);
+		    }
+		    break;
 		case '{':
 		    if (arg)
 			cc += arg;
diff --git a/Src/prompt.c b/Src/prompt.c
index 7acbe0e47..e10b05215 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -270,7 +270,8 @@ zattrescape(zattr atr, int *len)
 }
 
 /* Parse the argument for %H */
-static char *
+/**/
+mod_export char *
 parsehighlight(char *arg, char endchar, zattr *atr)
 {
     static int entered = 0;
@@ -295,9 +296,9 @@ parsehighlight(char *arg, char endchar, zattr *atr)
     } else
 	*atr = TXT_ERROR;
     if (ep)
-	*ep = endchar;
+	*ep++ = endchar;
     else
-	ep = strchr(arg, '\0') - 1;
+	ep = strchr(arg, '\0');
     entered = 0;
     return ep;
 }
@@ -635,6 +636,7 @@ putpromptchar(int doprint, int endchar)
 	    case 'H':
 		if (bv->fm[1] == '{') {
 		    bv->fm = parsehighlight(bv->fm + 2, '}', &atr);
+		    --bv->fm;
 		    if (atr != TXT_ERROR) {
 			treplaceattrs(atr);
 			applytextattributes(TSC_PROMPT);

Re: PATCH: broader support for highlight groups

[Bart Schaefer]

On Fri, Mar 1, 2024 at 5:21 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> We still lack a way to reset attributes. Rather than %r, which does
> have tenuous conflicts with completion explanations and some obscure
> uses of zformat, I'm tending towards special-casing %H{reset} and/or
> %H{none}.

This would somewhat be in line with the hash tables built by the
"colors" function ... which is both a plus and a minus, because I'm
already tired of explaining that prompts et al. do not use $color
internally.

> And perhaps disabling the feature of allowing left-over
> attributes in the prompt to bleed over into the user's input where those
> attributes came from a highlight group. That feature only exists for
> backward-compatibility and anyone using a new feature like highlight
> groups can also set the default key in zle_highlight. Any opinions?

If you choose to disable bleed-over, please have a look at the way
Functions/Misc/sticky-note applies its background coloring and adjust
if necessary?  I've never really spent any time understanding
zle_highlight, and although "vared" has options to set the prompts any
highlighting would (I think?) have to be done in the line-init and
line-finish widgets if the prompts can't bleed?

Re: PATCH: broader support for highlight groups

[Mikael Magnusson]

On 3/2/24, Oliver Kiddle <opk@zsh.org> wrote:
> This broadens the support for %H to completion explanation strings
> (with and without complist) and WATCHFMT, both of which already support
> %B/%S/%U/%F/%K. This also affects the list-prompt style as a
> side-effect.
>
> Completion for prompt strings also gains support for %H.
>
> The return value from parsehighlight() has been changed to go one
> past the end character because that's now convenient for more of the
> callers than leaving it on the end character. The one change of int to
> size_t in watch.c is to silence a (-Wextra) compiler warning.
>
> We still lack a way to reset attributes. Rather than %r, which does
> have tenuous conflicts with completion explanations and some obscure
> uses of zformat, I'm tending towards special-casing %H{reset} and/or
> %H{none}. And perhaps disabling the feature of allowing left-over
> attributes in the prompt to bleed over into the user's input where those
> attributes came from a highlight group. That feature only exists for
> backward-compatibility and anyone using a new feature like highlight
> groups can also set the default key in zle_highlight. Any opinions?

What about just %H without any arguments to reset?

-- 
Mikael Magnusson

sticky-note and prompt color leftovers (Re: PATCH: broader support for highlight groups)

[Oliver Kiddle]

On 1 Mar, Bart Schaefer wrote:
> If you choose to disable bleed-over, please have a look at the way
> Functions/Misc/sticky-note applies its background coloring and adjust
> if necessary?  I've never really spent any time understanding
> zle_highlight, and although "vared" has options to set the prompts any
> highlighting would (I think?) have to be done in the line-init and
> line-finish widgets if the prompts can't bleed?

Firstly, on the subject of sticky-note, had you intended to commit 47563
and/or 48773?

sticky-note uses raw escape sequences so it remains as broken as before.
For someone who does use zle_highlight, they get an initial clear to
end-of-line with the yellow background and then what they configured
applies for entered text. If that doesn't include a background the
yellow sticks around, at least until backspace, Ctrl-L or something
similar is pressed.

It doesn't need much more than to precede the vared with, e.g.
  local zle_highlight=( default:fg=black,bg=yellow )
Passing -p "%F{black}%K{yellow}" also works but I noticed one key
difference to the old bleed-over of raw escapes which is that they
aren't used for the initial clear to end-of-line sequence. I've attached
a patch which rectifies this but I won't be applying this as-is.
Backspace leaves default colours behind it - zsh is printing spaces
over erased characters. And this change also applies to clear to
end-of-display which I find a bit much.

I know that when I worked on this a year ago, I had it in mind to add
an additional key for zle_highlight which is used for line clearing
allowing a background for the area on the right that doesn't contain
text. I can't remember whether or not it was discussed. Any thoughts on
the name - viewport, buffer, field?

Limiting it to lines that contain text where we currently clear to end
of display might be tricky. Though that might please the person who
complained about that overwriting text they were randomly printing below
the cursor.

Oliver

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index f076bdd61..3f48e30dd 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -600,7 +600,8 @@ unset_region_highlight(Param pm, int exp)
 static void
 tcoutclear(int cap)
 {
-    cleartextattributes(0);
+    treplaceattrs(prompt_attr);
+    applytextattributes(0);
     tcout(cap);
 }
 

Re: sticky-note and prompt color leftovers (Re: PATCH: broader support for highlight groups)

[Bart Schaefer]

On Fri, Mar 29, 2024 at 6:06 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> Firstly, on the subject of sticky-note, had you intended to commit 47563
> and/or 48773?

No, but I was planning to commit 52314 if any feedback were provided
(which none has been, so far).  So please have a look a that, if you
don't mind.

> sticky-note uses raw escape sequences so it remains as broken as before.

Indeed, the ideal thing would be rewriting it to use highlight
instead.  (Not that I'm asking you to do more than provide pointers.)

> It doesn't need much more than to precede the vared with, e.g.
>   local zle_highlight=( default:fg=black,bg=yellow )

OK.  What about effects like underlining or blinking?

> I know that when I worked on this a year ago, I had it in mind to add
> an additional key for zle_highlight which is used for line clearing
> allowing a background for the area on the right that doesn't contain
> text. I can't remember whether or not it was discussed. Any thoughts on
> the name - viewport, buffer, field?

I don't recall discussion either.  How (if at all) does the PREDISPLAY
/ POSTDISPLAY text fit into this?  Would the fallback case be to match
the bg= color or to use the terminal default background?

Re: sticky-note and prompt color leftovers (Re: PATCH: broader support for highlight groups)

[Oliver Kiddle]

Bart Schaefer wrote:
> No, but I was planning to commit 52314 if any feedback were provided
> (which none has been, so far).  So please have a look a that, if you
> don't mind.

From the perspective of display attributes, that change would make it
significantly harder to adapt to using zle_highlight instead of the
colors function if you also need to maintain backward compatibility with
user style settings. In it's current form, with the associative array,
fg/bg can be mapped directly to fg= and bg= except where they have
values like bright-gray. The reset key could be ignored and the color
key would a problem.

The purpose of the display style might have been easier to follow if
you had used keys with names like urgent, later, work rather than none,
blink, reverse in the example. With the %s substitution form, prompt
strings would be more consistent with how we configure other things like
completion descriptons. And as %s turns off standout, %d or %n may be
better. I'd also be inclined to make use of the zstyle context, e.g.:

  zstyle ':sticky-note:urgent' display '%H{urgent}<<< %d >>>'

I don't personally like the existing approach of defining a style for
the fallback default. sticky-note currently does this for theme.

The recursive-edit doesn't seem to work too well, at least not starting
with my setup. I somehow need to Ctrl-C out of it. Might be related to
that ^M^M binding which is somehow hard to trigger despite KEYTIMEOUT
being generous enough. But if it works for you including with bindkey -v
then I probably need to bisect my setup.

Is there a way to remove or edit an existing note?

> > It doesn't need much more than to precede the vared with, e.g.
> >   local zle_highlight=( default:fg=black,bg=yellow )

Some of the other keys like paste, ellipsis and region may also be
applicable too.

> OK.  What about effects like underlining or blinking?

zle_highlight=( default:fg=black,bg=yellow,underline )

Would you want blink? Adding that to zle in the manner of italic and
faint would now be really easy but I didn't want to be too profligate
with the bits in zattr that got freed up in the refactoring. And my
normal terminal dropped support for blink at some point.

But given that terminals are now providing custom true-colour double
curly underline, we might have to find a way to remove the restrictions.

> > I know that when I worked on this a year ago, I had it in mind to add
> > an additional key for zle_highlight which is used for line clearing
> > allowing a background for the area on the right that doesn't contain
> > text. I can't remember whether or not it was discussed. Any thoughts on
> > the name - viewport, buffer, field?
>
> I don't recall discussion either.  How (if at all) does the PREDISPLAY
> / POSTDISPLAY text fit into this?  Would the fallback case be to match
> the bg= color or to use the terminal default background?

The background color would equally apply to those. There's no way for
widgets to control the cleared space and PREDISPLAY/POSTDISPLAY are
mostly used by widgets. This leads me to the conclusion that we would
instead need an extra key - clear, or clearbg, perhaps - so that you
would instead do:

  zle_highlight=( default:clear=yellow )

This doesn't need extra bits in zattr but would need an extra zattr for
each zle_highlight entry. I fear it would require consuming memory on a
colour for every position in the zle buffer. Perhaps, we need to think
about keeping indices into a palette instead. Maybe we can limit it to
default for now.

Oliver

Re: sticky-note and prompt color leftovers (Re: PATCH: broader support for highlight groups)

[Bart Schaefer]

On Sat, Mar 30, 2024 at 5:23 AM Oliver Kiddle <opk@zsh.org> wrote:
>
> Bart Schaefer wrote:
> > No, but I was planning to commit 52314 if any feedback were provided
>
> From the perspective of display attributes, that change would make it
> significantly harder to adapt to using zle_highlight instead of the
> colors function if you also need to maintain backward compatibility with
> user style settings.

There's no need for backward compatibility at this point, as that has
never gone anywhere except into 52314.  That implementation can be
scrapped and replaced with zle_highlight -compatible constructs.

The more complicated issue is how to apply the highlighting when doing
"list the existing sticky notes", because ZLE is not involved there.
I guess if it's limited to styling the entire note rather than
individual words, I only need the starting and ending %-expandos, if
there's an easy way to extract that from the $zle_highlight value.

> The purpose of the display style might have been easier to follow if
> you had used keys with names like urgent, later, work

I was copying the names originally proposed back in vapnik spaknik's
"Suggested improvement" thread (May 2021).

> [...]  as %s turns off standout, %d or %n may be
> better.

Could also just use a parameter interpolation, e.g. ${.zsticky.note}
... I used %s so that it could be passed to printf but that's not
really necessary, especially if I rewrite with ${|...}.

> I'd also be inclined to make use of the zstyle context, e.g.:
>
>   zstyle ':sticky-note:urgent' display '%H{urgent}<<< %d >>>'

That's complicated when it comes to listing out the possible context
choices for completion in _sticky_displays.  Suggestions?

> I don't personally like the existing approach of defining a style for
> the fallback default. sticky-note currently does this for theme.

This should be changed to use zstyle -q at the very least.  Hadn't got
there yet.

> The recursive-edit doesn't seem to work too well, at least not starting
> with my setup. I somehow need to Ctrl-C out of it. Might be related to
> that ^M^M binding which is somehow hard to trigger despite KEYTIMEOUT

If you've entered sticky-display with ^X? then you need one ^M to get
out of that and then two more rapidly to finish the note.  Does that
explain what you see?

> Is there a way to remove or edit an existing note?

Presently you can edit an existing note but then that is appended as a
new one.  Editing old notes would have to be done with e.g. "zed -h"
which I haven't integrated yet ... and has other issues because the
history timestamps are used to match up the notes with the display
file.

Otherwise sticky-note has to be completely rewritten to avoid use of
"fc -p" for the notefile, which is not out of the question but makes
up/down "history" of notes complicated.

> Would you want blink?

I would not, but vapnik did, which what got this started.  Could there
e.g. be a zattr bit that means "the next five bytes are a custom
attribute" and then just stuff in the escape sequence directly?  I
suppose that messes with remembering how attributes overlap.

> > I don't recall discussion either.  How (if at all) does the PREDISPLAY
> > / POSTDISPLAY text fit into this?  Would the fallback case be to match
> > the bg= color or to use the terminal default background?
>
> The background color would equally apply to those.

Perhaps a single boolean that says either to extend the background
color to the end of the line vs. reverting to the default terminal at
the end of the printable text, would be sufficient.  The only reason I
can think of for using something different would be to distinguish
trailing whitespace (i.e., where the background shows through but
there's actually something there in $BUFFER).