Search causes warning about isearch-search-fun-function scope

[ivanbrennan]

Upon starting Emacs in evil-mode, the first time I attempt a search (/), accessing the search history (M-p) causes a warning to display in the echo area:

Making isearch-search-fun-function local to *Minibuf-1* while let-bound!

Subsequent searches are unaffected. If the first search doesn't try to access search history, it too is unaffected.

I was able to repro with a minimal init.el

(package-initialize)
(require 'evil)
(add-hook 'after-init-hook #'evil-mode)

and the following steps:

• start Emacs
• type /
• type M-p

If I enable evil-mode manually instead of via after-init-hook, the warning never appears. Also, if I perform a normal isearch before performing the evil search, the warning never appears (neither for the isearch nor the evil search, regardless whether I access search history).

From what I can tell, the warning originates from make-local-variable and is being triggered in evil-search-incrementally when it let binds isearch-search-fun-function to evil-isearch-function. I was able to prevent the warning by removing that line, but I suspect that's not an ideal fix.

Screenshot:

[wasamasa]

Thanks for the bug report! I was initially skeptical of this as I couldn't find the warning in the *Messages* buffer, but managed reproducing it with your minimal init file.

Judging from some cursory sleuthing, the warning is an interaction of both Evil let-binding isearch-search-fun-function and isearch making the variable buffer-local whenever set with make-local-variable in minibuffer-history-isearch-setup. This function gets executed as part of minibuffer-setup-hook which is run every time the minibuffer is entered. That would explain why you only get to see this warning when doing very little beforehand, if you've used the minibuffer for M-x evil-mode or M-: (evil-mode), the code would have been run already and as the warning only shows up once, you won't ever see it again.

I'm not sure whether this issue can be solved in a meaningful way. Possible attempts:

• Not let-binding the isearch function. I'm not sure how you'd get proper integration in isearch otherwise, unless you decide to forego it completely...
• Executing some dummy thing beforehands. Well, nope, I'm not going to let that slide for a warning you hardly encounter
• Asking emacs-devel for cooperation on how to silence that warning. Potentially demoralizing, so beware! It's the only way I can see working though
• Not giving a damn and continuing with more important bugs.

@TheBB Thoughts?

[ivanbrennan]

@wasamasa I agree, this is a very low-impact bug. In case it helps, I came across a commit in ggtags that claims to fix the same sort of issue, though I'm not sure I fully understand the mechanics of it.

Also, this conversation seems to suggest a fix like,

diff --git a/evil-search.el b/evil-search.el
index db5c604..13660a1 100644
--- a/evil-search.el
+++ b/evil-search.el
@@ -76,6 +76,7 @@ search module is used."

 (defun evil-search-incrementally (forward regexp-p)
   "Search incrementally for user-entered text."
+  (make-local-variable 'isearch-search-fun-function)
   (let ((evil-search-prompt (evil-search-prompt forward))
         (isearch-search-fun-function 'evil-isearch-function)
         (point (point))

I tried that and it seemed to do the trick. I put in a PR (#750) with that change, but I'll defer to more experienced elispers as to whether this would have other, unintended consequences.

[wasamasa]

Well, that thread also suggests that any interaction of that kind is a reason to report a bug. This makes a lot of sense, why would the docstring of isearch-search-fun-function suggest it can be used to override search behavior, yet the usage of isearch from the minibuffer be problematic as its hook makes the variable become buffer-local? I suspect we're either missing out on some implementation detail (isearch doesn't use the minibuffer to my knowledge) or someone went for the improper solution.

In other words, I will not merge your PR for now and report a bug instead.

edit: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25561

[ivanbrennan]

@wasamasa Would you prefer I close the PR or wait to see if anything comes of the bug report?

[wasamasa]

Please wait for at least a few days and give them a chance to respond.

[ivanbrennan]

Will do, thanks.

[wasamasa]

I've CC'd Stefan today and, well, see for yourself: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25561#17

Reads to me as if it's not worth bothering.

[ninrod]

sounds like he is considering removing the warning in a future emacs version, but he does not give any certainty about that.

[wasamasa]

Wow, I didn't expect this bug to result in the complete removal of that class of warnings: http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=73ea77c856ded90cfb1a03a9d87827b5ecb93a7c

This means both this bug and the PR can be closed. Thanks for making this happen, @ivanbrennan!