treat zle-isearch-exit like an accept-* widget

[m0vie]

When hitting ENTER in an isearch, the cursor imprint needs to be removed, and any path_prefix highlighting needs to be removed.

See 4f0c293 and 59fbdda.

Does it make sense to keep this a a user-configurable setting? The pattern matching is a bit complex.
It is ony useful if the user has custom widgets that call accpt-* internally. But then one could argue, that the custom widget name should start with accept-, too.

[danielshahaf]

Does it make sense to keep this a a user-configurable setting? The pattern matching is a bit complex.

My initial thought is: YAGNI. Let's just have a helper function that does [[ $1 == accept-* ]] || [[ $1 == zsh-isearch-exit ]] and add the additional complexity only when needed.

How do you get path_prefix highlighting while in isearch mode?

[m0vie]

My initial thought is: YAGNI. Let's just have a helper function that does [[ $1 == accept-* ]] || [[ $1 == zsh-isearch-exit ]] and add the additional complexity only when needed.

Done.

How do you get path_prefix highlighting while in isearch mode?
Not while in there, but after

touch fooooooo
vim foo

underlining of foo gets removed when pressing enter.

<^R>vim foo<ENTER>

Underlining stays because accepting an isearch does not call through any of the accept- widgets. However it calls zle-isearch-exit.

[danielshahaf]

Thanks, I can reproduce the issue now. However, I can also reproduce the issue under zsh -f with your patch applied. I suspect your patch only works when a zsh-isearch-exit widget had been created before z-sy-h was loaded.

Is it even possible to write a regression test for this? I'm not sure how hard it'd be to trigger this codepath; would it be as smple as having the test-data/*.zsh file set WIDGET=zsh-isearch-exit? (Test files are isolated from each other: doing this won't affect other test files.)

[danielshahaf]

Cross-referencing issue #261: this issue might be easier to solve after the redrawhook work (in zsh and in z-sy-h) is done, including the isearch caveat mentioned in #261.

[m0vie]

Oh, you are right. As long as there is no zsh-isearch-exit widget loaded, this patch is not useful at all.

In a clean zsh -f environment the only widget that is being called when pressing RETURN in an isearch is history-incremental-search-backward.

In the case where zle-isearch-* widgets are defined before loading z-s-h, the order in which widgets are called is this:

zle-isearch-update
zle-isearch-exit
history-incremental-search-backward

So it looks like history-* should be added to the accept-type list, instead of zle-isearch-exit?

[danielshahaf]

So it looks like history-* should be added to the accept-type list, instead of zle-isearch-exit?

It sounds odd that history-incremental-search-backward is called after zle-isearch-exit. How did you determine the order? Is there any chance that the calltree was:

_zsh_highlight_widget_-history-incremental-search-backward
- .history-incremental-search-backward [built-in widget]
  - zle-isearch-update
    - debug print $WIDGET
  - zle-isearch-exit
    - debug print $WIDGET
- _zsh_highlight
  - debug print $WIDGET

??

[danielshahaf]

Assuming zle-isearch-exit is indeed the last widget invoked by pressing in isearch mode, what we'd want is for z-sy-h to run code at that point. Conceptually, we'd want to add-zsh-hook zle-isearch-exit _zsh_highlight_isearch_hook [pseudocode!].

Alternatively, perhaps this could be fixed through the redrawhook facility; that would require fixing the upstream issue with redrawhook and isearch (#261 (comment)) and merging one of the redrawhook-based backend implementations (#245).

(And please raise that issue on -workers@ — if the redrawhook work has issues the best time to diagnose/fix them is before it's released.)

[danielshahaf]

workers/37590 invokes redrawhook from the isearch codepath, which would allow to fix this issue once the z-sy-h uses redrawhook (#245).

[m0vie]

This is the wrong approach for the issue.

The proper way to handle these special cases is to always bind zle-line-finish and use this as indicator.

I'll re-open another pull request with a proper fix.

[m0vie]

Opened issue #284 to track this further.