Highlight style is not cleared on accept when manually set to a value <8

MithicSpirit commented 1 year ago

Describe the bug

If ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE is manually set where fg or bg are less than 8, then the highlighting is not reset when accepting or partially accepting the suggestion.

To Reproduce

% zsh -df
% source path/to/zsh-autosuggestions.zsh
% ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE='fg=1'
% ZSH<right arrow key>

(instead of typing <right arrow key> press the right arrow key on your keyboard to accept the autosuggestion, which should be _AUTOSUGGEST_HIGHLIGHT_STYLE='fg=1' in red text). At this point, ZSH should be in the regular color but _AUTOSUGGEST_HIGHLIGHT_STYLE='fg=1' (which was completed) will still be in red.

Note that similar results occur for fg=x or bg=x when x<8, or a combination thereof (fg=x,bg=y where either x or y are <8).

Expected behavior

The color of _AUTOSUGGEST_HIGHLIGHT_STYLE='fg=1' would be reset, like when the variable is unchanged or set to a value >=8.

Screenshots

Note how the cursor (yellow vertical bar) is ahead of the red text, so it has retained the color even though it was accepted.

Desktop

OS + distribution: Arch Linux
Zsh version: 5.9
Plugin version: a411ef3e0992d4839f0732ebeb9823024afaaaa8
Terminal: alacritty 0.11.0-dev (ed4614d0)

Additional context

If I remember correctly, this issue began when I updated to zsh 5.9. I'm not exactly sure whether it was that update, but I know I didn't use to have this issue and started having it sort of recently (a few weeks or months ago).

xgqt commented 1 year ago

Same problem on Gentoo Linux, ZSH version 5.9.

Only workaround for now for me was to change foreground color id to more than 8; 14 looked similar enough to 6 :)

GannonTdW commented 1 year ago

Hello, Same problem here.

zsh 5.9
alacritty 0.10.1-2
gnome-terminal 3.44.1-1
terminator 2.1.1-3

gfxcc commented 1 year ago

Same problem after zsh being updated to 5.9.

ivan commented 1 year ago

zsh master up to https://github.com/zsh-users/zsh/commit/ab4d62eb975a4c4c51dd35822665050e2ddc6918 also has this issue. I looked through the commits from 5.8.1 to 5.9 and I didn't find anything obviously color-related.

MithicSpirit commented 1 year ago

I guess bisecting zsh might be required?

ivan commented 1 year ago

https://github.com/zsh-users/zsh/commit/0721060f3616deac84f82c4a97d75987e276fe0a bad https://github.com/zsh-users/zsh/commit/714864a87b11b080da92dee7abc29b8bc64107a5 good

I confirmed that reverting https://github.com/zsh-users/zsh/commit/0721060f3616deac84f82c4a97d75987e276fe0a fixes the reset of zsh-autosuggestion's region highlight for colors < 8.

Hi-Angel commented 1 year ago

@ivan thank you for the bisect! Did you report it to zsh?

ivan commented 1 year ago

I did not report it.

okapia commented 1 year ago

I'm the author of the zsh patch you bisected this down to. @danielshahaf alerted me to this issue. If you read the mailing list message associated with that patch, I said the following:

From my testing, the one arguable regression due to this is that when assigning to region_highlight, we now lose the information of whether a colour was specified in named or numbered form so fg=2 gets converted to fg=green. Conversion to a canonical form seems harmless enough, or? And if we only want to preserve the input form then the flag and associated variables should be named differently.

This affects the following code used several times in zsh-autosuggestions:

region_highlight=("${(@)region_highlight:#$_ZSH_AUTOSUGGEST_LAST_HIGHLIGHT}")

This is changing just the entries in region_highlight that are due to zsh-autosuggestions. Fortunately, in 5.9 Daniel added a separate feature to make it easier for plugins to change region_highlight without clobbering the entries from other plugins. Entries can now optionally include a string of the form memo=token which is preserved verbatim. Just search for the word memo in zshzle(1). For an example, I used the feature in https://github.com/okapia/zsh-viexchange/blob/master/exchange That does also workaround the fact that the memo feature is lacking older zsh releases but there may be better approaches. I think the exact form I used in vi-exchange with the space was chosen because 5.8 silently drops the memo rather than failing - region_highlight+=( " 0 0 fg=red, $memo" )

I believe that the main crux of the patch in 47510 is correct, valid and fixes other issues so reverting it is not something I would favour.

HofiOne commented 1 year ago

Same issue here

zsh - 5.9
iTerm2 - 3.4.17
macOS 13.0

Any chance to get this fixed?

Could the usage of $memo everywhere the _ZSH_AUTOSUGGEST_LAST_HIGHLIGHT is referenced be a solution? man zshzle(1) says Note that zsh 5.8 and older do not support the "memo=token" field and may misparse the third (highlight specification) field when a memo is given.

HofiOne commented 1 year ago

ok, meanwhile I double-checked, and yes, 5.9 ignores the "memo=token"

It turned out, in my case, the problem was that I set my ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE to custom color code with CAPITAL case hexadecimal values (#A1A2A6) and seems the region_highlight array handling (now?) converts to lower case everything that added to it, therefore the _zsh_autosuggest_highlight_reset() function could not remove the earlier set value via the used region_highlight=("${(@)region_highlight:#$_ZSH_AUTOSUGGEST_LAST_HIGHLIGHT_SAVE}") expression

so, take care, either

use only lowercase letters when you set ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE via a hexadecimal color code, or
as a dirty hack you can replace _zsh_autosuggest_highlight_apply with this tiny modification in zsh-autosuggestions.zsh

_zsh_autosuggest_highlight_apply() {
    typeset -g _ZSH_AUTOSUGGEST_LAST_HIGHLIGHT_SAVE

    if (( $#POSTDISPLAY )); then
        # Make sure the possibly user defined (or anytime meanwhile changed ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE is lower case,
        # it seems region_highlight items are now all lower case converted
        # NOTE: This quick hack is not a real solution as
        #       - might not be backward compatible
        #       - might not be future compatible
        # TODO: convert both the region_highlight items and the ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE to the same letter case
        ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE=${ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE:l}

        typeset -g _ZSH_AUTOSUGGEST_LAST_HIGHLIGHT_SAVE="$#BUFFER $(($#BUFFER + $#POSTDISPLAY)) $ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE"

        region_highlight+=("$_ZSH_AUTOSUGGEST_LAST_HIGHLIGHT_SAVE")
    else
        unset _ZSH_AUTOSUGGEST_LAST_HIGHLIGHT_SAVE
    fi
}

okapia commented 1 year ago

It turned out, in my case, the problem was that I set my ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE to custom color code with CAPITAL case hexadecimal values (#A1A2A6)

5.8 wasn't preserving the exact format either, the values go into a packed internal structure. When reconstituted, the attributes get sorted so fg= comes first followed by bg and then other attributes, initial zeroes are stripped from cursor positions and, as you observe, the case of hex values is not preserved. The fact that 5.8 preserved the distinction between e.g. "red" and "1" for the first 8 colours was more by accident than design.

memo should be preserved verbatim in 5.9 because that was what it is intended for. The value can't contain commas but otherwise should remain unadulterated.

HofiOne commented 1 year ago

thanks for the explanation! we agree it should be preserved, but actually, this does not happen :( is it reported already as a bug to the zsh developers?

danielshahaf commented 1 year ago

@HofiOne If I understand @okapia (the author of the zsh commit) correctly, he's saying zsh-autosuggestions relies on an implementation detail of zsh, which has changed. Y'all are welcome to report this to the zsh mailing list to see if folks there think differently.

I'd recommend z-asug be changed to DTRT under zsh 5.9. z-asug can probe for this feature with something along the lines of:

region_highlight+=( "0 0 bg=1, memo=zsh-autosuggestions-test" )
case ${region_highlight[-1]} in
  …
esac
region_highlight[-1]=()

and then keep the current code for zsh≤5.8 and use a new, memo-based implementation for 5.9. (5.9 will give bg=red memo=…; 5.8 will give bg=1 and discard the memo.)

matthewtusker commented 1 year ago

I don't use zsh-syntax-highlighting and have this issue too, but not when ZSH_AUTOSUGGEST_HIGHLIGHT_STYLE is set to bold or underline. Setting fg or bg with any other option prevents the style from clearing.

matthewtusker commented 1 year ago

To follow up on this, I've done a bit more testing and it appears that the styling won't reset after accepting a suggestion if either fg or bg is <=7, regardless of other styling. As soon as either is below 8 then the styling persists.

MithicSpirit commented 1 year ago

it appears that the styling won't reset after accepting a suggestion if either fg or bg is <=7, regardless of other styling.

Yeah sorry if that wasn't clear from the bug report

zsh-users / zsh-autosuggestions