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

[protist]

I recently upgraded zsh from 5.2-2 -> 5.3.1-1, in Arch Linux. After upgrading, history-incremental-pattern-search-backward and history-incremental-pattern-search-forward no longer work. That is, they show the most recent match, then failing bck-i-search: foo, then one other match from eons ago, then it fails to find any more. Reverting to zsh 5.2-2 fixes this problem.

I use zim. When I disable zsh-syntax-highlighting, pattern searching works again. I tried creating a new minimal zsh install, that did not source zim at all. As soon as I directly source zsh-syntax-highlighting, pattern searching breaks again. Unlike issue #387, I'm not using zsh-autocorrect.

[danielshahaf]

I use [zim](https://github.com/Eriner/zim). When I disable `zsh-syntax-highlighting`, pattern searching works again. I tried creating a new minimal zsh install, that did not source `zim` at all. As soon as I directly source `zsh-syntax-highlighting`, pattern searching breaks again. Unlike issue #387, I'm *not* using `zsh-autocorrect`.

I cannot reproduce this. zsh -f bindkey ^R history-incremental-pattern-search-backward source ./zsh-syntax-highlighting.zsh : foo : bar : baz <^R>ba?<^R><^R><^R> Can you please post a minimal example starting from `zsh -f`? With as few third party modules as possible, please.

[protist]

Oddly enough, your example fails for me!

$ zsh -f
% bindkey ^R history-incremental-pattern-search-backward
% source ./zsh-syntax-highlighting.zsh 
% foo
zsh: command not found: foo
% bar
zsh: command not found: bar
% baz
zsh: command not found: baz

I type <^R>ba?

% baz
bck-i-search: ba?_

Then again <^R>

% baz
failing bck-i-search: ba?_

And it's stuck on baz, and never gets to bar.

I tested (from the start) searching with ba instead of ba?, which also fails. I then tested ba using the default history-incremental-search-backward, which works as expected.

I wasn't previously familiar with zsh -f, but my understanding is that it skips all initiation files except for the system-wide zshenv. However, I don't have any file on my system named that anyway. I'm not sure how else my system might differ from yours.

[danielshahaf]

And it's stuck on `baz`, and never gets to `bar`. I tested (from the start) searching with `ba` instead of `ba?`, which also fails. I then tested `ba` using the default `history-incremental-search-backward`, which works as expected.

So, to clarify, you're saying that a search for just `ba` with pattern search finds baz but not bar?

I wasn't previously familiar with `zsh -f`, but my understanding is that it skips all initiation files except for the system-wide `zshenv`.

Correct.

However, I don't have any file on my system named that anyway. I'm not sure how else my system might differ from yours.

Distros sometimes change the path of zshenv to /etc/zsh/zshenv or similar. What does `zsh -fx` print before the first prompt? What versions of everything do you use? I use: % print -rl -- $ZSH_VERSION $ZSH_PATCHLEVEL $ZSH_HIGHLIGHT_VERSION $ZSH_HIGHLIGHT_REVISION 5.3.1-dev-0 zsh-5.3.1-1-gfa88f57 0.6.0-dev aac4a44 Does your distro patch zsh? Do you have more than one zsh binary on your system?

[protist]

So, to clarify, you're saying that a search for just ba with pattern search finds baz but not bar?

Correct.

Distros sometimes change the path of zshenv to /etc/zsh/zshenv or similar.

Sorry, I should have been more specific. locate zshenv comes up with nothing. Also, there's nothing at /etc/zsh/zshenv.

What does zsh -fx print before the first prompt?

Nothing at all.

What versions of everything do you use?

% 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?

Does your distro patch zsh?

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

Do you have more than one zsh binary on your system?

No, just the one.

[danielshahaf]

``` % 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 aac4a44 ``` 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](https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/zsh), 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 aac4a44 but with that plus a local mod: ```diff 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)?

[m0vie]

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.

[danielshahaf]

This is actually an upstream bug.

Thanks for the quick resposne and analysis.

Minimal example without z-sy-h to reproduce:

: ⋮

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

I can reproduce that on 5.3.1.

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 ***@***.***

Wonderful. (It hasn't appeared in the archives yet so can't link to it.)

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?

Can we dynamically probe for the bug? E.g., is there some . a=(); : $a[(r)foo*]; [[ bar = … ]]; echo $? . construct that gives different results with and without the upstream patch? Even if we wanted to fix this only for history-incremental-pattern-search-backward, and even if we had access to the pattern entered in the minibuffer, testing whether $BUFFER matches that pattern would not suffice (it would be a one-sided test, specific but not sensitive, e.g., if the history had been `: foo bar', `: bar', `: baz' in your example)).

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.

"Luckily", aye. But that's bound to change... (and let's not say anything about signal handlers firing asynchronously)

Of course we'd lose z-sy-h's coloring again until then.

If we have to choose between correctness and coloring, we should choose correctness. Thanks again for getting to the bottom of this.

[m0vie]

Fixed in upstream with
zsh-users/zsh@48cadf4
http://www.zsh.org/mla/workers//2017/msg00034.html

[danielshahaf]

Fixed in upstream with zsh-users/zsh@48cadf4 http://www.zsh.org/mla/workers//2017/msg00034.html

That should be included in upstream 5.3.2 or 5.4 (whichever comes first). Have you made progress on probing for this bug at runtime, or some other workaround for ≤5.3.1?

[danielshahaf]

Further discussion on http://www.zsh.org/cgi-bin/mla/redirect?WORKERNUMBER=40328 (same thread, but the web archive doesn't join it)

[protist]

Have you made progress on probing for this bug at runtime, or some other workaround for ≤5.3.1?

Thank you for the quick fix upstream, but is there a way to get it working in the meantime? Thanks in advance.