GNU bug report logs -
#43120
28.0.50; fido-mode: M-j before completions appear selects wrong choice
Previous Next
Reported by: Sean Whitton <spwhitton <at> spwhitton.name>
Date: Sun, 30 Aug 2020 21:01:01 UTC
Severity: normal
Tags: fixed
Merged with 43083
Found in version 28.0.50
Fixed in version 28.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 43120 in the body.
You can then email your comments to 43120 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
larsi <at> gnus.org, joaotavora <at> gmail.com, bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sun, 30 Aug 2020 21:01:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Sean Whitton <spwhitton <at> spwhitton.name>
:
New bug report received and forwarded. Copy sent to
larsi <at> gnus.org, joaotavora <at> gmail.com, bug-gnu-emacs <at> gnu.org
.
(Sun, 30 Aug 2020 21:01:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Hello,
The recent change to fix #19032 has broken my fido-mode workflow in a
way which I think reveals a bug.
When starting from an empty minibuffer, icomplete-fido-exit now selects
a different match depending on whether the completions have appeared
yet. So if you hit keys fast enough then you get different behaviour
than if you hit them a bit slower.
For example:
1. emacs -q
2. M-x fido-mode RET
3. C-x b foo M-j (type slowly)
4. C-x b bar M-j (type slowly)
5. C-x b M-j (type quickly)
This will take you to *GNU Emacs* but it should take you to foo (the
minibuffer prompt is "Switch to buffer (default foo)"). If you wait for
the completions to appear before hitting M-j, you go to the right place.
I think the fix would be to clear completion-content-when-empty when the
minibuffer exits, instead of leaving data from the last completion
there. Or possibly M-j should call icomplete-completions to popular
completion-content-when-empty with the correct information.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sun, 30 Aug 2020 23:51:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> For example:
>
> 1. emacs -q
> 2. M-x fido-mode RET
> 3. C-x b foo M-j (type slowly)
> 4. C-x b bar M-j (type slowly)
> 5. C-x b M-j (type quickly)
This sounds like a bug, indeed. I dealt with this kind of problem in
the past and worked hard to mitigate it. I was unware of this recent
patch to icomplete/fido-mode, sorry.
I will note that the problem doesn't seem to happen if step 5 ends with
RET instead of M-j. M-j in the fido-mode minibuffer is bound to
icomplete-fido-exit which does have have an optional FORCE arg, which
could maybe justify the behaviour you're observing. But anyway it's nil
by default, so it doesn't.
> I think the fix would be to clear completion-content-when-empty when the
> minibuffer exits, instead of leaving data from the last completion
> there. Or possibly M-j should call icomplete-completions to popular
> completion-content-when-empty with the correct information.
I don't understand the reason of being for the patch. I didn't read the
bug, but I do read this in NEWS:
+*** 'icomplete-show-matches-on-no-input' behavior change
+Previously, choosing a different completion with commands like 'C-.'
+and then hitting enter would choose the default completion. Doung
+this will now choose the completion under point.
Now, I have never observed this reported behaviour in fido-mode even
before the patch.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Mon, 31 Aug 2020 16:38:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Hello João,
On Mon 31 Aug 2020 at 12:50AM +01, João Távora wrote:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> For example:
>>
>> 1. emacs -q
>> 2. M-x fido-mode RET
>> 3. C-x b foo M-j (type slowly)
>> 4. C-x b bar M-j (type slowly)
>> 5. C-x b M-j (type quickly)
>
> This sounds like a bug, indeed. I dealt with this kind of problem in
> the past and worked hard to mitigate it. I was unware of this recent
> patch to icomplete/fido-mode, sorry.
>
> I will note that the problem doesn't seem to happen if step 5 ends with
> RET instead of M-j. M-j in the fido-mode minibuffer is bound to
> icomplete-fido-exit which does have have an optional FORCE arg, which
> could maybe justify the behaviour you're observing. But anyway it's nil
> by default, so it doesn't.
Right. I actually have icomplete-fido-exit bound to RET because that
means that RET in any minibuffer always exits with the current input,
which I find makes completion less obstrusive. But this bug is breaking
that atm.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Tue, 01 Sep 2020 14:17:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> I don't understand the reason of being for the patch. I didn't read the
> bug, but I do read this in NEWS:
>
> +*** 'icomplete-show-matches-on-no-input' behavior change
> +Previously, choosing a different completion with commands like 'C-.'
> +and then hitting enter would choose the default completion. Doung
> +this will now choose the completion under point.
>
> Now, I have never observed this reported behaviour in fido-mode even
> before the patch.
It was reported as a problem in icomplete, but I don't know how fido
relates to icomplete.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Tue, 01 Sep 2020 14:20:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> I think the fix would be to clear completion-content-when-empty when the
> minibuffer exits, instead of leaving data from the last completion
> there.
Oh yeah, definitely -- it should be cleared on minibuffer exit. I'm
guessing that should go in... `completion--complete-and-exit'? Would
it be possible for you to propose a patch that does that? I don't use
fido-mode myself, and it's probably easier for you to test whether
that's the right fix.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Tue, 01 Sep 2020 18:29:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> João Távora <joaotavora <at> gmail.com> writes:
>
>> I don't understand the reason of being for the patch. I didn't read the
>> bug, but I do read this in NEWS:
>>
>> +*** 'icomplete-show-matches-on-no-input' behavior change
>> +Previously, choosing a different completion with commands like 'C-.'
>> +and then hitting enter would choose the default completion. Doung
>> +this will now choose the completion under point.
>>
>> Now, I have never observed this reported behaviour in fido-mode even
>> before the patch.
>
> It was reported as a problem in icomplete, but I don't know how fido
> relates to icomplete.
This makes sense then. fido-mode is basically icomplete-mode with
different defaults and slightly different bindings, resulting in a
visual experience that looks vaguely like Ido (Fido = "fake Ido").
However, the description of this icomplete problem sounds like something
where fido-mode and icomplete-mode should exhibit exactly the same
behaviour, i.e. work along the very same code. So I'm baffled as to why
that problem affected icomplete and not fido-mode. Almost equally
baffled as to why it now breaks fido-mode.
I see in the side thread there seems to be a straightforward and logical
fix. If that makes sense globally and also fixes it for Sean, I'm fine
with it, otherwise I'd suggest having a second look at the original
problem that prompted the problematic patch, specifically at why
fido-mode didn't suffer from it.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 12:41:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> I see in the side thread there seems to be a straightforward and logical
> fix. If that makes sense globally and also fixes it for Sean, I'm fine
> with it, otherwise I'd suggest having a second look at the original
> problem that prompted the problematic patch, specifically at why
> fido-mode didn't suffer from it.
It didn't, but it sure does now.
I just noticed that this affects bare-bones fido-mode as well, i.e. even
without using M-j we get some akward and very highly annoying cache.
In an Emacs -Q with fido-mode enabled:
(bound-and-true-p [press C-h f RET here, slowly])
now move the point to the "setq" in, say
(setq blabla )
And type C-h f RET here, quickly. You'll be presented with
`bound-and-true-p`'s doc, not `setq`'s.
This is a regression: the bug should be fixed ASAP. Maybe some
"clearing" as was suggested is in order. I'm not very confortable
writing that patch right now. Can anyone else do it? Or should we
revert the original fix instead in the meantime?
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 13:12:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> This is a regression: the bug should be fixed ASAP. Maybe some
> "clearing" as was suggested is in order. I'm not very confortable
> writing that patch right now. Can anyone else do it? Or should we
> revert the original fix instead in the meantime?
I think the fix should be pretty trivial. I don't have time to test it
today, but tomorrow should be open.
Reverting the problematic patch in the meantime is fine by me, but on
the other hand, it'll get reapplied again tomorrowish, I guess, so that
seems like unnecessary churn...
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 13:14:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Thu, Sep 3, 2020 at 2:11 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
> João Távora <joaotavora <at> gmail.com> writes:
>
> > This is a regression: the bug should be fixed ASAP. Maybe some
> > "clearing" as was suggested is in order. I'm not very confortable
> > writing that patch right now. Can anyone else do it? Or should we
> > revert the original fix instead in the meantime?
>
> I think the fix should be pretty trivial. I don't have time to test it
> today, but tomorrow should be open.
>
> Reverting the problematic patch in the meantime is fine by me, but on
> the other hand, it'll get reapplied again tomorrowish, I guess, so that
> seems like unnecessary churn...
No problem then, but can you give me an idea of the patch, so
I can hotfix my Emacs master where this is bothering me a bit?
João
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 13:52:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> No problem then, but can you give me an idea of the patch, so
> I can hotfix my Emacs master where this is bothering me a bit?
I think setting the variable to nil in completion--complete-and-exit is
probably the correct fix, but I haven't tested it.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 13:57:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Ok, thanks. I'll try that.
On Thu, Sep 3, 2020 at 2:51 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
> João Távora <joaotavora <at> gmail.com> writes:
>
> > No problem then, but can you give me an idea of the patch, so
> > I can hotfix my Emacs master where this is bothering me a bit?
>
> I think setting the variable to nil in completion--complete-and-exit is
> probably the correct fix, but I haven't tested it.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
> bloggy blog: http://lars.ingebrigtsen.no
>
--
João Távora
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 03 Sep 2020 23:25:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Hello Lars,
On Tue 01 Sep 2020 at 04:19PM +02, Lars Ingebrigtsen wrote:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> I think the fix would be to clear completion-content-when-empty when the
>> minibuffer exits, instead of leaving data from the last completion
>> there.
>
> Oh yeah, definitely -- it should be cleared on minibuffer exit. I'm
> guessing that should go in... `completion--complete-and-exit'? Would
> it be possible for you to propose a patch that does that? I don't use
> fido-mode myself, and it's probably easier for you to test whether
> that's the right fix.
I'm certainly happy to test a patch but I don't think that can be the
right place to clear the variable.
The docstring for that function says it exits from a require-match
minibuffer. But completion gets used in other minibuffers too. So we
need to clear the variable somewhere which gets called for every
minibuffer exit.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Fri, 04 Sep 2020 02:13:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> The docstring for that function says it exits from a require-match
> minibuffer. But completion gets used in other minibuffers too. So we
> need to clear the variable somewhere which gets called for every
> minibuffer exit.
Yeah, following the exit logic (from the multiple number of code paths)
isn't trivial here. But as far as I can tell, the exit-minibuffer
function is called in all the relevant code paths?
Does this fix the problem for you?
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 864726e3cc..8984440576 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2062,6 +2062,9 @@ exit-minibuffer
;; (or to turn it into a list of buffers, ...), but in the mean time,
;; this should do the trick in most cases.
(setq deactivate-mark nil)
+ ;; Clear any computed default values (so that they're not used on
+ ;; the next invocation).
+ (setq completion-content-when-empty nil)
(throw 'exit nil))
(defun self-insert-and-exit ()
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Fri, 04 Sep 2020 03:23:01 GMT)
Full text and
rfc822 format available.
Message #44 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Fri 04 Sep 2020 at 04:11AM +02, Lars Ingebrigtsen wrote:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> The docstring for that function says it exits from a require-match
>> minibuffer. But completion gets used in other minibuffers too. So we
>> need to clear the variable somewhere which gets called for every
>> minibuffer exit.
>
> Yeah, following the exit logic (from the multiple number of code paths)
> isn't trivial here. But as far as I can tell, the exit-minibuffer
> function is called in all the relevant code paths?
>
> Does this fix the problem for you?
Unfortunately it does not, so I guess icomplete-fido-exit is not exiting
the minibuffer through that path.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Fri, 04 Sep 2020 03:28:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> Unfortunately it does not, so I guess icomplete-fido-exit is not exiting
> the minibuffer through that path.
Hm. Am I misreading the code here? So the else branch definitely calls
exit-minibuffer...
(defun icomplete-fido-exit (force)
[...]
(if (and (not force) minibuffer--require-match)
(minibuffer-complete-and-exit)
(exit-minibuffer)))
And the "then" branch ends up here:
(defun minibuffer-complete-and-exit ()
[...]
(completion-complete-and-exit (minibuffer-prompt-end) (point-max)
#'exit-minibuffer))
Hm... which then calls completion--complete-and-exit, which should then
end up calling the exit function in... hm. Perhaps not all the
branches? There's a bunch of callbacks, but I thought I followed them
all to the end and they all ended up calling the exit function, but
perhaps not? Would it be possible for you to edebug through
completion-complete-and-exit?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Fri, 04 Sep 2020 14:48:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Hello Lars,
On Fri 04 Sep 2020 at 05:27AM +02, Lars Ingebrigtsen wrote:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> Unfortunately it does not, so I guess icomplete-fido-exit is not exiting
>> the minibuffer through that path.
>
> Hm. Am I misreading the code here? So the else branch definitely calls
> exit-minibuffer...
>
> (defun icomplete-fido-exit (force)
> [...]
> (if (and (not force) minibuffer--require-match)
> (minibuffer-complete-and-exit)
> (exit-minibuffer)))
>
> And the "then" branch ends up here:
>
> (defun minibuffer-complete-and-exit ()
> [...]
> (completion-complete-and-exit (minibuffer-prompt-end) (point-max)
> #'exit-minibuffer))
>
> Hm... which then calls completion--complete-and-exit, which should then
> end up calling the exit function in... hm. Perhaps not all the
> branches? There's a bunch of callbacks, but I thought I followed them
> all to the end and they all ended up calling the exit function, but
> perhaps not? Would it be possible for you to edebug through
> completion-complete-and-exit?
I think I see what the problem is. I was doing the following to
generate a test case: C-h f comp [wait for completions to appear] C-g
This leaves completion-content-when-empty populated to interfere with
the next run, as getting out of the minibuffer that way does not clear
the variable, but it should.
Might it work to set the variable buffer-local to the minibuffer? Then
we can be sure it would always be cleared.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sat, 05 Sep 2020 12:27:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> I think I see what the problem is. I was doing the following to
> generate a test case: C-h f comp [wait for completions to appear] C-g
Oh, right; didn't think that through properly...
> This leaves completion-content-when-empty populated to interfere with
> the next run, as getting out of the minibuffer that way does not clear
> the variable, but it should.
>
> Might it work to set the variable buffer-local to the minibuffer? Then
> we can be sure it would always be cleared.
All this time I thought the minibuffer was reused, but poking around a
bit now, it seems like it's not?
In which case -- does the following fix this problem?
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 8a68df876c..ba266cfbfe 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -715,7 +715,7 @@ icomplete-completions
(setq prospects (nreverse prospects))
;; Return the first match if the user hits enter.
(when icomplete-show-matches-on-no-input
- (setq completion-content-when-empty (car prospects)))
+ (setq-local completion-content-when-empty (car prospects)))
;; Decorate first of the prospects.
(when prospects
(let ((first (copy-sequence (pop prospects))))
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sat, 05 Sep 2020 15:33:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Hello,
On Sat 05 Sep 2020 at 02:25PM +02, Lars Ingebrigtsen wrote:
> In which case -- does the following fix this problem?
It does indeed. Hope this patch can be applied.
--
Sean Whitton
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sat, 05 Sep 2020 21:13:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Sean Whitton <spwhitton <at> spwhitton.name> writes:
> On Sat 05 Sep 2020 at 02:25PM +02, Lars Ingebrigtsen wrote:
>
>> In which case -- does the following fix this problem?
>
> It does indeed. Hope this patch can be applied.
Thanks for testing; I've now applied the patch.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) fixed.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Sat, 05 Sep 2020 21:13:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 28.1, send any further explanations to
43120 <at> debbugs.gnu.org and Sean Whitton <spwhitton <at> spwhitton.name>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Sat, 05 Sep 2020 21:13:02 GMT)
Full text and
rfc822 format available.
Forcibly Merged 43083 43120.
Request was from
Sean Whitton <spwhitton <at> spwhitton.name>
to
control <at> debbugs.gnu.org
.
(Sun, 06 Sep 2020 00:06:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Sun, 06 Sep 2020 18:28:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> On Sat 05 Sep 2020 at 02:25PM +02, Lars Ingebrigtsen wrote:
>>
>>> In which case -- does the following fix this problem?
>>
>> It does indeed. Hope this patch can be applied.
>
> Thanks for testing; I've now applied the patch.
I've had a look at the original problem that triggered this, and I
wonder if this much simpler patch wouldn't be preferable. For one, it
doesn't touch the minibuffer.el machinery (which is complicated as it
is) or has any kind of complicated caching semantics. It just binds a
different command to RET in icomplete-minibuffer-map, presumably solving
19032 (in my limited testing). It's also guaranteed not to affect
fido-mode.
I think something like this is the way to go for a behaviour change such
as this.
João
[0001-Change-icomplete-show-matches-on-no-input-behaviour-.patch (text/x-diff, inline)]
From f4dc81e0c7be75ace3766ca16e2be8bdcc8f0627 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Sun, 6 Sep 2020 19:03:52 +0100
Subject: [PATCH] Change icomplete-show-matches-on-no-input behaviour for
Icomplete only
Fixes: bug#19032, bug#43120
Previous fixes to bug#19032 introduced bugs in Fido mode. This fix
relies on a new command bound to RET.
* etc/NEWS (Miscellaneous): Mention icomplete-show-matches-on-no-input.
* lisp/icomplete.el (icomplete-show-matches-on-no-input): Add comment.
(icomplete-minibuffer-map): Bind icomplete-ret.
(icomplete-ret): New command.
---
etc/NEWS | 6 ++++++
lisp/icomplete.el | 16 +++++++++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/etc/NEWS b/etc/NEWS
index 749b28ac3f..d40a4807ec 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -938,6 +938,12 @@ window after starting). This variable defaults to nil.
** Miscellaneous
+---
+*** 'icomplete-show-matches-on-no-input' behavior change
+Previously, choosing a different completion with commands like 'C-.'
+and then hitting enter would choose the default completion. Doing
+this will now choose the completion under point.
+
+++
*** The user can now customize how "default" values are prompted for.
The new utility function 'format-prompt' has been added which uses the
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index f76ab28fb8..c4d5012af9 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -75,7 +75,11 @@ icomplete-tidy-shadowed-file-names
selection process starts again from the user's $HOME.")
(defcustom icomplete-show-matches-on-no-input nil
- "When non-nil, show completions when first prompting for input."
+ "When non-nil, show completions when first prompting for input.
+This also means that if you traverse the list of completions with
+commands like `C-.' and just hit RET without typing any
+characters, the match under point will be chosen instead of the
+default."
:type 'boolean
:version "24.4")
@@ -153,12 +157,22 @@ icomplete-post-command-hook
(defvar icomplete-minibuffer-map
(let ((map (make-sparse-keymap)))
(define-key map [?\M-\t] 'icomplete-force-complete)
+ (define-key map (kbd "RET") 'icomplete-ret)
(define-key map [?\C-j] 'icomplete-force-complete-and-exit)
(define-key map [?\C-.] 'icomplete-forward-completions)
(define-key map [?\C-,] 'icomplete-backward-completions)
map)
"Keymap used by `icomplete-mode' in the minibuffer.")
+(defun icomplete-ret ()
+ "Exit minibuffer for icomplete."
+ (interactive)
+ (if (and icomplete-show-matches-on-no-input
+ (car completion-all-sorted-completions)
+ (eql (icomplete--field-end) (icomplete--field-beg)))
+ (icomplete-force-complete-and-exit)
+ (exit-minibuffer)))
+
(defun icomplete-force-complete-and-exit ()
"Complete the minibuffer with the longest possible match and exit.
Use the first of the matches if there are any displayed, and use
--
2.25.1
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Mon, 07 Sep 2020 10:31:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> I've had a look at the original problem that triggered this, and I
> wonder if this much simpler patch wouldn't be preferable. For one, it
> doesn't touch the minibuffer.el machinery (which is complicated as it
> is) or has any kind of complicated caching semantics. It just binds a
> different command to RET in icomplete-minibuffer-map, presumably solving
> 19032 (in my limited testing). It's also guaranteed not to affect
> fido-mode.
That does look like a much simpler and less invasive way to implement
this; yes. (And you'd presumably remove the stuff that was added for
19032 already?)
But I was wondering whether there were any other use cases where the
newly added stuff would be useful... but perhaps not?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Mon, 07 Sep 2020 17:31:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 43120 <at> debbugs.gnu.org (full text, mbox):
> I think something like this is the way to go for a behaviour change such
> as this.
Looks fine to me.
> + (define-key map (kbd "RET") 'icomplete-ret)
Maybe use a `remap`ping instead?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Tue, 08 Sep 2020 06:53:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> I think something like this is the way to go for a behaviour change such
>> as this.
>
> Looks fine to me.
>
>> + (define-key map (kbd "RET") 'icomplete-ret)
>
> Maybe use a `remap`ping instead?
Yes, maybe makes sense.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Tue, 08 Sep 2020 09:01:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> I think something like this is the way to go for a behaviour change such
>> as this.
>
> Looks fine to me.
Thanks, pushed.
>> + (define-key map (kbd "RET") 'icomplete-ret)
>
> Maybe use a `remap`ping instead?
I did that, too.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 14:02:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 43120 <at> debbugs.gnu.org (full text, mbox):
João Távora <joaotavora <at> gmail.com> writes:
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>> I think something like this is the way to go for a behaviour change such
>>> as this.
>>
>> Looks fine to me.
>
> Thanks, pushed.
>
>>> + (define-key map (kbd "RET") 'icomplete-ret)
>>
>> Maybe use a `remap`ping instead?
>
> I did that, too.
> +(defun icomplete-ret ()
> + "Exit minibuffer for icomplete."
> + (interactive)
> + (if (and icomplete-show-matches-on-no-input
> + (car completion-all-sorted-completions)
> + (eql (icomplete--field-end) (icomplete--field-beg)))
> + (icomplete-force-complete-and-exit)
> + (exit-minibuffer)))
This changed the behavior of RET from `minibuffer-complete-and-exit' to
`exit-minibuffer'. Was that intention? What I noticed is the following.
[before]
emacs -Q
M-x icomplete-mode
C-xd /usr
C-xk u ;; shows "Kill buffer (default usr): u(sr)"
RET
killed "usr" buffer
[after]
emacs -Q
M-x icomplete-mode
C-xd /usr
C-xk u ;; shows "Kill buffer (default usr): u(sr)"
RET
No buffer named u
Thanks.
--
OGAWA Hirofumi <hirofumi <at> mail.parknet.co.jp>
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 16:13:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Wed, Sep 9, 2020 at 3:01 PM OGAWA Hirofumi <hirofumi <at> mail.parknet.co.jp>
wrote:
> João Távora <joaotavora <at> gmail.com> writes:
>
> > Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> >
> >>> I think something like this is the way to go for a behaviour change
> such
> >>> as this.
> >>
> >> Looks fine to me.
> >
> > Thanks, pushed.
> >
> >>> + (define-key map (kbd "RET") 'icomplete-ret)
> >>
> >> Maybe use a `remap`ping instead?
> >
> > I did that, too.
>
> > +(defun icomplete-ret ()
> > + "Exit minibuffer for icomplete."
> > + (interactive)
> > + (if (and icomplete-show-matches-on-no-input
> > + (car completion-all-sorted-completions)
> > + (eql (icomplete--field-end) (icomplete--field-beg)))
> > + (icomplete-force-complete-and-exit)
> > + (exit-minibuffer)))
>
> This changed the behavior of RET from `minibuffer-complete-and-exit' to
> `exit-minibuffer'. Was that intention?
>
Nope, sorry. You're right. It should read minibuffer-complete-and-exit there
of course.
João
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 17:53:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 43120 <at> debbugs.gnu.org (full text, mbox):
> Nope, sorry. You're right. It should read minibuffer-complete-and-exit there
> of course.
IIRC this depends on whether the completion is `require-match` or not.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 19:05:03 GMT)
Full text and
rfc822 format available.
Message #92 received at 43120 <at> debbugs.gnu.org (full text, mbox):
>> +(defun icomplete-ret ()
>> + "Exit minibuffer for icomplete."
>> + (interactive)
>> + (if (and icomplete-show-matches-on-no-input
>> + (car completion-all-sorted-completions)
>> + (eql (icomplete--field-end) (icomplete--field-beg)))
>> + (icomplete-force-complete-and-exit)
>> + (exit-minibuffer)))
>
> This changed the behavior of RET from `minibuffer-complete-and-exit' to
> `exit-minibuffer'. Was that intention? What I noticed is the following.
I confirm that this change broke icomplete-mode. Here is a test case
that shows regression:
M-x icomplete-mode RET
M-x rgr RET
Lisp error: (error "‘rgr’ is not a valid command name")
Until yesterday it used to run 'rgrep'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 19:14:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> Nope, sorry. You're right. It should read minibuffer-complete-and-exit there
>> of course.
>
> IIRC this depends on whether the completion is `require-match` or not.
If so, shouldn't minibuffer-complete-and-exit take care of that? I
mean, I've remapped the binding to _that_ command, so unless it's making
some "(interactive)" magic, which it is not, calling it from lisp in the
normal case as it should be completely equivalent.
I've gone ahead and commited the fix, as it fixes Hirofumi's problem, as
he described it.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 19:53:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 43120 <at> debbugs.gnu.org (full text, mbox):
>>> Nope, sorry. You're right. It should read minibuffer-complete-and-exit there
>>> of course.
>> IIRC this depends on whether the completion is `require-match` or not.
> If so, shouldn't minibuffer-complete-and-exit take care of that?
Yes and no: IIRC depending on `require-match`, RET is bound either
to `minibuffer-complete-and-exit` or to `exit-minibuffer`.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 19:55:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Wed, Sep 9, 2020 at 8:52 PM Stefan Monnier <monnier <at> iro.umontreal.ca>
wrote:
> >>> Nope, sorry. You're right. It should read minibuffer-complete-and-exit
> there
> >>> of course.
> >> IIRC this depends on whether the completion is `require-match` or not.
> > If so, shouldn't minibuffer-complete-and-exit take care of that?
>
> Yes and no: IIRC depending on `require-match`, RET is bound either
> to `minibuffer-complete-and-exit` or to `exit-minibuffer`.
Bah, so the remap you suggested wouldn't work anyway. What to do?
The good 'ol :filter trick? How does it go again?
João
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 19:59:02 GMT)
Full text and
rfc822 format available.
Message #104 received at 43120 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Wed, Sep 9, 2020 at 8:54 PM João Távora <joaotavora <at> gmail.com> wrote:
> On Wed, Sep 9, 2020 at 8:52 PM Stefan Monnier <monnier <at> iro.umontreal.ca>
> wrote:
>
>> >>> Nope, sorry. You're right. It should read
>> minibuffer-complete-and-exit there
>> >>> of course.
>> >> IIRC this depends on whether the completion is `require-match` or not.
>> > If so, shouldn't minibuffer-complete-and-exit take care of that?
>>
>> Yes and no: IIRC depending on `require-match`, RET is bound either
>> to `minibuffer-complete-and-exit` or to `exit-minibuffer`.
>
>
> Bah, so the remap you suggested wouldn't work anyway. What to do?
> The good 'ol :filter trick? How does it go again?
>
Alternatively (and a bit sillily), two remaps for two different commands:
one for exit-minibuffer and one for minibuffer-complete-and-exit. Or
check minibuffer-require-match which was recently added. Pick your
poison.
João
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 20:37:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 43120 <at> debbugs.gnu.org (full text, mbox):
> Alternatively (and a bit sillily), two remaps for two different commands:
> one for exit-minibuffer and one for minibuffer-complete-and-exit.
I'd go with that, yes,
Also, because it will handle the case where the user has added a binding
to `minibuffer-complete-and-exit` to the keymap where RET is bound to
`exit-minibuffer`.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Wed, 09 Sep 2020 22:09:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> Alternatively (and a bit sillily), two remaps for two different commands:
>> one for exit-minibuffer and one for minibuffer-complete-and-exit.
>
> I'd go with that, yes,
>
> Also, because it will handle the case where the user has added a binding
> to `minibuffer-complete-and-exit` to the keymap where RET is bound to
> `exit-minibuffer`.
OK, how's this look?
Though I'm starting to think that when require-match is nil, an
icomplete user wouldn't want the new icomplete-show-matches-on-no-input
behaviour anyway. But I'm not one of those. Else, if she does, doesn't
it mean she wants fido-mode instead?
The question is thus: remap exit-minibuffer or not? It means usually:
exit with whatever has been input, which may well be the empty string.
João
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 4e546807b7..6d48aa84d4 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -157,21 +157,31 @@ icomplete-post-command-hook
(defvar icomplete-minibuffer-map
(let ((map (make-sparse-keymap)))
(define-key map [?\M-\t] 'icomplete-force-complete)
- (define-key map [remap minibuffer-complete-and-exit] 'icomplete-ret)
+ (define-key map [remap minibuffer-complete-and-exit] 'icomplete-complete-and-exit)
+ (define-key map [remap exit-minibuffer] 'icomplete-exit)
(define-key map [?\C-j] 'icomplete-force-complete-and-exit)
(define-key map [?\C-.] 'icomplete-forward-completions)
(define-key map [?\C-,] 'icomplete-backward-completions)
map)
"Keymap used by `icomplete-mode' in the minibuffer.")
-(defun icomplete-ret ()
- "Exit minibuffer for icomplete."
- (interactive)
+(defun icomplete--maybe-force (fallback)
+ "Helper for `icomplete-complete-and-exit' and `icomplete-exit'."
(if (and icomplete-show-matches-on-no-input
(car completion-all-sorted-completions)
(eql (icomplete--field-end) (icomplete--field-beg)))
(icomplete-force-complete-and-exit)
- (minibuffer-complete-and-exit)))
+ (funcall fallback)))
+
+(defun icomplete-complete-and-exit ()
+ "Complete, then exit minibuffer for icomplete."
+ (interactive)
+ (icomplete--maybe-force #'minibuffer-complete-and-exit))
+
+(defun icomplete-exit ()
+ "Exit minibuffer for icomplete."
+ (interactive)
+ (icomplete--maybe-force #'exit-minibuffer))
(defun icomplete-force-complete-and-exit ()
"Complete the minibuffer with the longest possible match and exit.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 10 Sep 2020 04:37:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 43120 <at> debbugs.gnu.org (full text, mbox):
> The question is thus: remap exit-minibuffer or not?
Good question. I think if there is a default value (i.e. if
exit-minibuffer would return that non-nil default when the minibuffer
is empty), then I think it makes sense to use the new
icomplete-show-matches-on-no-input, but if not, indeed we probably
should return "" (otherwise we're making it impossible to return "").
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 10 Sep 2020 18:54:02 GMT)
Full text and
rfc822 format available.
Message #116 received at 43120 <at> debbugs.gnu.org (full text, mbox):
> Though I'm starting to think that when require-match is nil, an
> icomplete user wouldn't want the new icomplete-show-matches-on-no-input
> behaviour anyway.
This is confusing, I don't understand why behaviour of
icomplete-show-matches-on-no-input should depend on require-match.
Here are two examples that produce a different result.
The first example is from hi-lock-read-face-name:
(defvar hi-history nil)
(icomplete-mode)
(let ((icomplete-show-matches-on-no-input t)
(defaults '("hi-yellow" "hi-green"))
(hi-history '("hi-blue")))
(completing-read
(format-prompt "Highlight using face" (car defaults))
obarray 'facep t nil 'hi-history defaults))
displays this prompt:
Highlight using face (default hi-yellow): {link | menu | bold ...
Typing RET returns "link" (and sometimes returns "hi-blue" from the history),
but never returns the expected default value "hi-yellow".
Whereas the second example from tab-bar-switch-to-tab
works correctly since its arg require-match is nil:
(let ((icomplete-show-matches-on-no-input t)
(defaults '("yellow" "green"))
(hi-history '("blue")))
(completing-read
(format-prompt "Switch to tab by name" (car defaults))
defaults nil nil nil 'hi-history defaults))
displays this prompt:
Switch to tab by name (default yellow): {green | yellow}
Typing RET returns the default value "yellow", not the first candidate "green".
This makes the behaviour of icomplete-show-matches-on-no-input
unpredictable, and thus in some cases dangerous.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#43120
; Package
emacs
.
(Thu, 10 Sep 2020 19:16:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 43120 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
>> Though I'm starting to think that when require-match is nil, an
>> icomplete user wouldn't want the new icomplete-show-matches-on-no-input
>> behaviour anyway.
>
> This is confusing, I don't understand why behaviour of
> icomplete-show-matches-on-no-input should depend on require-match.
>
> Here are two examples that produce a different result.
>
> The first example is from hi-lock-read-face-name:
>
> (defvar hi-history nil)
> (icomplete-mode)
> (let ((icomplete-show-matches-on-no-input t)
> (defaults '("hi-yellow" "hi-green"))
> (hi-history '("hi-blue")))
> (completing-read
> (format-prompt "Highlight using face" (car defaults))
> obarray 'facep t nil 'hi-history defaults))
>
> displays this prompt:
>
> Highlight using face (default hi-yellow): {link | menu | bold ...
>
> Typing RET returns "link" (and sometimes returns "hi-blue" from the history),
> but never returns the expected default value "hi-yellow".
>
> Whereas the second example from tab-bar-switch-to-tab
> works correctly since its arg require-match is nil:
>
> (let ((icomplete-show-matches-on-no-input t)
> (defaults '("yellow" "green"))
> (hi-history '("blue")))
> (completing-read
> (format-prompt "Switch to tab by name" (car defaults))
> defaults nil nil nil 'hi-history defaults))
>
> displays this prompt:
>
> Switch to tab by name (default yellow): {green | yellow}
>
> Typing RET returns the default value "yellow", not the first candidate "green".
>
> This makes the behaviour of icomplete-show-matches-on-no-input
> unpredictable, and thus in some cases dangerous.
I think I agree, but I've just tested this with the version of
icomplete.el before I started messing with this stuff (commit
c8472cc69d4bce7f53c9a62966245a4de3d99fbd) and I get exactly the same
results as you. So I'd leave my work here for someone else to pick up
on: To be clear, I just wanted to simplify/refactor the code to be less
intrusive on minibuffer.el.
I'm not much of an icomplete-mode user, more of a fido-mode user where
these discrepancies are "fixed" by copying ido-mode's behaviour.
João
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Fri, 09 Oct 2020 11:24:07 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 311 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.