zsh-syntax-highlighting: history-incremental-pattern-search-backward is broken after upgrading to zsh 5.3.1

Fixed in upstream with https://github.com/zsh-users/zsh/commit/48cadf48ff9c61ce09e826ad9a58e250e999a262 http://www.zsh.org/mla/workers//2017/msg00034.html

Thank you for the update.

No worries.

Debian has also backported…

I’m mortified that Debian was quicker than Arch with this! 😧

@protist Correct. The argument for 5.3.1.1 over 5.3.2 is that it would be correct even if zsh started using x.y.z.w. The arguments for 5.3.2 over 5.3.1.1 are that it suffices given upstream’s release numbering policy (as you say) and that it’s better to be conservative due to the asymmetric failure modes (as pointed out by @docwhat).

I’m not sure I have a good suggestion here. Would it work for you to just locally remove the && ! (( $+ISEARCHMATCH_ACTIVE )) clause?

Oh yes, sorry. For some reason I forgot to try that. Yes, that certainly works; thank you. Are there any negative ramifications for removing this?

During isearch, there will be no highlighting at all.

You’ll want to remember to revert your local mod when you upgrade to zsh≥5.3.2.

This is actually an upstream bug.

Minimal example without z-sy-h to reproduce:

% bindkey '^R' history-incremental-pattern-search-backward
% evil_hook() { a=(); : ${a[(r)foo*]}; };
% zle -N zle-isearch-update evil_hook
% : foo
% : bar
% : baz
%

type: <^R>b
% : baz
bck-i-search: b_

type: <^R>
% : foo
bck-i-search: b_

': foo' is found instead of ': bar' because evil_hook modified the
static pattern used in isearch.

The cause is that iseach uses the global PAT_STATIC pattern. But inside the zle-isearch-hook any function that uses patterns, too, will overwrite that pattern.

I sent a patch to workers@.

This bug got introduced in #288, because we deliberately hooked zle-isearch-update to fix another bug. If a user would have hooked zle-isearch-update himself already, the bug would have already surfaced before.

Question now is: Can we do anything on z-sy-h’s side to prevent triggering this?

This (pseudocode) should technically do it,

-  if [[ $WIDGET == zle-isearch-update ]] && ! (( $+ISEARCHMATCH_ACTIVE )); then
+  if [[ $WIDGET == zle-isearch-update ]] && ! zsh_bug_is_fixed; then

since we (luckily) don’t use any patterns before it. Of course we’d lose z-sy-h’s coloring again until then.

% print -rl -- $ZSH_VERSION $ZSH_PATCHLEVEL $ZSH_HIGHLIGHT_VERSION $ZSH_HIGHLIGHT_REVISION
5.3.1
zsh-5.3.1-0-g06b1b7a
0.6.0-dev
aac4a4423898fccbd1ab72c369ac09995a9139f1

Looks like the zsh version is slightly different to yours, but I am using the latest release. I can try the latest -git release if you think that might be more consistent with yours?

There’s no need; the only difference between zsh-5.3.1-0-g06b1b7a and zsh-5.3.1-1-gfa88f57 is a patch that changes the version number reporting. (zsh-users/zsh@fa88f57)

Does your distro patch zsh?

It looks like it does some minimal patching, but I’m not sure if this has any effect.

Nothing there accounts for the difference.

sigh

I see the problem. I was testing not with aac4a4423898fccbd1ab72c369ac09995a9139f1 but with that plus a local mod:

diff --git a/zsh-syntax-highlighting.zsh b/zsh-syntax-highlighting.zsh
index 0251d1e..cce6669 100644
--- a/zsh-syntax-highlighting.zsh
+++ b/zsh-syntax-highlighting.zsh
@@ -67,7 +67,7 @@ _zsh_highlight()
 
   # Remove all highlighting in isearch, so that only the underlining done by zsh itself remains.
   # For details see FAQ entry 'Why does syntax highlighting not work while searching history?'.
-  if [[ $WIDGET == zle-isearch-update ]] && ! (( $+ISEARCHMATCH_ACTIVE )); then
+  if [[ $WIDGET == zle-isearch-update ]]; then
     region_highlight=()
     return $ret
   fi

I can reproduce your behaviour when I revert that change.

That change is a rebase artifact from one of @movie’s pull requests (#288 or thereabouts; the referenced FAQ entry — “Does syntax highlighting work during incremental history search?” in README.md — points to that issue, too).

@m0vie, do you recall the background here? Did @protist’s use-case work at some point during 0.5.0 development? Can it be made to work (without disabling syntax highlighting)?