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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Andrew Schwartzmeyer
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


0001-Fix-prompts-when-using-fido-mode.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
> 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.



Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
Juri Linkov <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
>> 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.



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
In reply to this post by Andrew Schwartzmeyer
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?


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)
Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
Juri Linkov <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Juri Linkov-2
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.