bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Andrew Schwartzmeyer]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

When fido-mode is enabled, you need to use M-j to end the selection, not RET. This is similar to ido-mode, which was already special-cased for multi-occur and multi-isearch-buffers.

The attached patch fixes these two instances, however I’ve discovered two more bugs while testing it. For one, M-j does NOT work in multi-isearch-files, and I’m not sure why. It’s supposed to work; it works for the other two. So I didn’t update the prompt in that function (which, by the way is not special cased for ido-mode,

For two, the check for ido-mode is actually broken, but I don’t yet know how to fix it. It does (eq read-buffer-function #'ido-read-buffer), but actually nowadays (maybe this was different in the past), ido-mode doesn’t set read-buffer-function, unless you call ido-everywhere, and then it still doesn’t set it but overrides it with an advice, so that its value is #f(advice-wrapper :override nil ido-read-buffer), and this fails the eq test.

Anyway, I’d suggest applying this patch for now, and then figuring out how to fix the check for ido.

Thanks,

Andy


[-- Attachment #2: 0001-Fix-prompts-when-using-fido-mode.patch --]
[-- Type: application/octet-stream, Size: 2420 bytes --]

From 86c663ae22845a075cea9216a4a5e053e6c16c04 Mon Sep 17 00:00:00 2001
From: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
Date: Sun, 31 May 2020 11:56:01 -0700
Subject: [PATCH] Fix prompts when using fido-mode

When fido-mode is enabled, you need to use M-j to end the selection, not
RET. This is similar to ido-mode, which was already special-cased for
multi-occur and multi-isearch-buffers.
---
 lisp/misearch.el | 11 ++++++++---
 lisp/replace.el  | 11 ++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lisp/misearch.el b/lisp/misearch.el
index 958c10a1b..69ba77fea 100644
--- a/lisp/misearch.el
+++ b/lisp/misearch.el
@@ -237,9 +237,14 @@ set in `multi-isearch-buffers' or `multi-isearch-buffers-regexp'."
 	 (ido-ignore-item-temp-list bufs))
     (while (not (string-equal
 		 (setq buf (read-buffer
-			    (if (eq read-buffer-function #'ido-read-buffer)
-				"Next buffer to search (C-j to end): "
-			      "Next buffer to search (RET to end): ")
+			    (concat
+                             "Next buffer to search "
+                             (cond
+                              ((eq read-buffer-function #'ido-read-buffer)
+                               "(C-j to end): ")
+                              ((bound-and-true-p fido-mode)
+                               "(M-j to end): ")
+                              (t "(RET to end): ")))
 			    nil t))
 		 ""))
       (add-to-list 'bufs buf)
diff --git a/lisp/replace.el b/lisp/replace.el
index 69092c16f..8cd92d894 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1585,9 +1585,14 @@ See also `multi-occur-in-matching-buffers'."
 	   (ido-ignore-item-temp-list bufs))
       (while (not (string-equal
 		   (setq buf (read-buffer
-			      (if (eq read-buffer-function #'ido-read-buffer)
-				  "Next buffer to search (C-j to end): "
-				"Next buffer to search (RET to end): ")
+			      (concat
+                               "Next buffer to search "
+                               (cond
+                                ((eq read-buffer-function #'ido-read-buffer)
+                                 "(C-j to end): ")
+                                ((bound-and-true-p fido-mode)
+                                 "(M-j to end): ")
+                                (t "(RET to end): ")))
 			      nil t))
 		   ""))
 	(cl-pushnew buf bufs)
-- 
2.26.0

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[João Távora]

Andrew Schwartzmeyer <andrew@schwartzmeyer.com> writes:

> Anyway, I’d suggest applying this patch for now, and then figuring out
> how to fix the check for ido.

Hi Andrew,

Thanks for the report.  I don't have anything to comment here
specifically, since I'm not familiar with the modes that you propose
patching, so let's wait for someone more knowledgeable with those modes.

However, at first sight, it doesn't seem to be particularly off the
mark, except that a user might change that binding and break it.  Maybe
a better, more generic fix would be ask the current minibuffer for the
answer, perhaps by searching the keymap for the keybinding that maps to
a command symbol that has a certain property set on it (and then
fido-mode and ido-mode would play ball).  But maybe that's too
complex...

João

PS: It's fine to include me in your bug report to call my attention
(indeed I encourage you to do so).  However, the manner in which you
must do so is a bit counter-intuitive: you should avoid "CC:" in the
mail you send to bug-gnu-emacs@gnu.org, because if you do and I then
"reply to all", it'll just cause another bug report, and not the effect
you intend.  If your mail client allows it, use "X-Debbugs-CC:" instead.
Alternatively, you can forward me the email that you get (usually within
minutes) from <bug-number>@debbugs.gnu.org.

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Juri Linkov]

> Thanks for the report.  I don't have anything to comment here
> specifically, since I'm not familiar with the modes that you propose
> patching, so let's wait for someone more knowledgeable with those modes.

I don't know who is more knowledgeable since I just blindly
copied that ugly ‘(eq read-buffer-function #'ido-read-buffer)’
from multi-occur to multi-isearch-read-buffers.

Also I see that the patch that Andrew sent works fine in multi-isearch-files.

> However, at first sight, it doesn't seem to be particularly off the
> mark, except that a user might change that binding and break it.  Maybe
> a better, more generic fix would be ask the current minibuffer for the
> answer, perhaps by searching the keymap for the keybinding that maps to
> a command symbol that has a certain property set on it (and then
> fido-mode and ido-mode would play ball).  But maybe that's too
> complex...

Something like this is needed indeed, not too complicated.
If it's not easy to just rephrase the prompt to avoid mentions of
the key, then I suggest to display the string returned from

  (substitute-command-keys "(\\[exit-minibuffer] to end): ")

called in the minibuffer.

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Something like this is needed indeed, not too complicated.
> If it's not easy to just rephrase the prompt to avoid mentions of
> the key, then I suggest to display the string returned from
>
>   (substitute-command-keys "(\\[exit-minibuffer] to end): ")
>
> called in the minibuffer.

Here's a stupid question -- how do you do that?  :-)  I looked around for
a primitive to eval something in the minibuffer, but I can't find one.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Juri Linkov]

>> Something like this is needed indeed, not too complicated.
>> If it's not easy to just rephrase the prompt to avoid mentions of
>> the key, then I suggest to display the string returned from
>>
>>   (substitute-command-keys "(\\[exit-minibuffer] to end): ")
>>
>> called in the minibuffer.
>
> Here's a stupid question -- how do you do that?  :-)  I looked around for
> a primitive to eval something in the minibuffer, but I can't find one.

Sorry, I meant just

  (substitute-command-keys "(\\<icomplete-fido-mode-map>\\[icomplete-fido-exit] to end): ")
  => "(M-j to end): "

not necessarily called in the minibuffer, but with the minibuffer's keymap.

A more complex solution like proposed by João would be to set a certain
property set on a command's symbol that exits the minibuffer.

But maybe 'cond' in Andrew's path with an additional substitute-command-keys
(for the case when the user remaps the default keys) is fine.

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Sorry, I meant just
>
>   (substitute-command-keys
> "(\\<icomplete-fido-mode-map>\\[icomplete-fido-exit] to end): ")
>   => "(M-j to end): "
>
> not necessarily called in the minibuffer, but with the minibuffer's keymap.

Ah, I'd forgotten about the \\<icomplete-fido-mode-map> construct.

And then the actual minibuffer map isn't relevant...  or is it?

Anyway, I factored this out into multi-occur--prompt (and used it from
both call sites), and it seems to work well for me in both fido and ido
modes when calling multi-occur.

Pushed to Emacs 28 now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

reopen 41633
quit

> For two, the check for ido-mode is actually broken, but I don’t yet know
> how to fix it. It does (eq read-buffer-function #'ido-read-buffer), but
> actually nowadays (maybe this was different in the past), ido-mode doesn’t
> set read-buffer-function, unless you call ido-everywhere, and then it still
> doesn’t set it but overrides it with an advice, so that its value is
> #f(advice-wrapper :override nil ido-read-buffer), and this fails the
> eq test.
>
> Anyway, I’d suggest applying this patch for now, and then figuring out
> how to fix the check for ido.

I confirm it doesn't work with ido-everywhere.
Maybe this is a better condition?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ido-everywhere.patch --]
[-- Type: text/x-diff, Size: 439 bytes --]

diff --git a/lisp/replace.el b/lisp/replace.el
index 4883ecfc8f..b717a2a25c 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1588,7 +1588,7 @@ multi-occur--prompt
   (concat
    "Next buffer to search "
    (cond
-    ((eq read-buffer-function #'ido-read-buffer)
+    ((bound-and-true-p ido-everywhere)
      (substitute-command-keys
       "(\\<ido-completion-map>\\[ido-select-text] to end): "))
     ((bound-and-true-p fido-mode)

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> I confirm it doesn't work with ido-everywhere.
> Maybe this is a better condition?
>
> diff --git a/lisp/replace.el b/lisp/replace.el
> index 4883ecfc8f..b717a2a25c 100644
> --- a/lisp/replace.el
> +++ b/lisp/replace.el
> @@ -1588,7 +1588,7 @@ multi-occur--prompt
>    (concat
>     "Next buffer to search "
>     (cond
> -    ((eq read-buffer-function #'ido-read-buffer)
> +    ((bound-and-true-p ido-everywhere)

Makes sense, I think -- I think pretty much the only way to have ido
enabled in these two functions is to have ido-anywhere enabled?  On the
other hand, perhaps somebody bound read-buffer-function "manually" here,
so perhaps something like

    (or (eq read-buffer-function #'ido-read-buffer)
        (bound-and-true-p ido-everywhere))

would be slightly more correct?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41633: Prompts incorrect for multi-occur and multi-isearch when using fido-mode

[Juri Linkov]

tags 41633 fixed
close 41633 28.0.50
quit

>> @@ -1588,7 +1588,7 @@ multi-occur--prompt
>>    (concat
>>     "Next buffer to search "
>>     (cond
>> -    ((eq read-buffer-function #'ido-read-buffer)
>> +    ((bound-and-true-p ido-everywhere)
>
> Makes sense, I think -- I think pretty much the only way to have ido
> enabled in these two functions is to have ido-anywhere enabled?  On the
> other hand, perhaps somebody bound read-buffer-function "manually" here,
> so perhaps something like
>
>     (or (eq read-buffer-function #'ido-read-buffer)
>         (bound-and-true-p ido-everywhere))
>
> would be slightly more correct?

Ok, let's use both.  So closing this again.