GNU bug report logs - #34116
27.0.50; minibuffer-force-complete-and-exit mostly broken

Previous Next

Package: emacs;

Reported by: João Távora <joaotavora <at> gmail.com>

Date: Thu, 17 Jan 2019 13:57:02 UTC

Severity: normal

Tags: patch

Found in version 27.0.50

Done: João Távora <joaotavora <at> gmail.com>

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 34116 in the body.
You can then email your comments to 34116 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Thu, 17 Jan 2019 13:57:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to João Távora <joaotavora <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 17 Jan 2019 13:57:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: monnier <at> iro.umontreal.ca
Subject: 27.0.50; minibuffer-force-complete-and-exit mostly broken
Date: Thu, 17 Jan 2019 13:56:39 +0000
[Message part 1 (text/plain, inline)]
Hi maintainers,

Here's a cute one from my recent icomplete adventures,

  Emacs -Q
  M-x icomplete-mode
  M-: (completing-read "test: " '("foo" "bar"))
  C-M-i ;; minibuffer-force-complete, this completes to "foo"
  C-j   ;; icomplete-force-complete-and-exit

Expected "foo", right?  Nope: you get "bar".

Incidently, this also happens without icomplete, provided you have bound
the commands minibuffer-force-complete and
minibuffer-force-complete-and-exit in minibuffer-local-completion-map.

The problem happens because m-f-b cycles/rotates silently rotates the
candidate list _after_ forcing the completion.  The other second
"and-exit" command, which also calls m-f-b during its execution, catches
this extra rotation before returning the final value to the user.

The attached patch fixes this.  It add considerable hackery to an
already nasty collection of hacks, but is the safest way short of
redesigning this whole cycling business (which is heavily used in
completion-at-point).

This patch also simplifies the fix for bug#34077 which I had previously
submitted.

João

[0001-Fix-nasty-cycling-on-minibuffer-force-complete-and-e.patch (text/x-patch, inline)]
From 97756513d4fe4521f3eff5376ad1ecfce5e1f266 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 16 Jan 2019 23:29:34 +0000
Subject: [PATCH] Fix nasty cycling on minibuffer-force-complete-and-exit

minibuffer-force-complete sets up cycling after forcing the
completion, which is mostly fine for successive interactive calls.
However minibuffer-force-complete-and-exit also calls it.  In
situations where the former and latter are called in succession this
causes an unwanted extra cycle, which ultimately yields the wrong
completion.

* lisp/minibuffer.el (minibuffer-force-complete): Take DONT-CYCLE
arg.
(minibuffer-force-complete-and-exit): Pass DONT-CYCLE to
minibuffer-force-complete.
---
 lisp/minibuffer.el | 79 +++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5760a2e49d..74f85e7b90 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1257,29 +1257,32 @@ completion-all-sorted-completions
 (defun minibuffer-force-complete-and-exit ()
   "Complete the minibuffer with first of the matches and exit."
   (interactive)
-  (minibuffer-force-complete)
+  (minibuffer-force-complete nil nil t)
   (completion--complete-and-exit
    (minibuffer-prompt-end) (point-max) #'exit-minibuffer
    ;; If the previous completion completed to an element which fails
    ;; test-completion, then we shouldn't exit, but that should be rare.
    (lambda () (minibuffer-message "Incomplete"))))
 
-(defun minibuffer-force-complete (&optional start end)
-  "Complete the minibuffer to an exact match.
-Repeated uses step through the possible completions."
+(defun minibuffer-force-complete (&optional start end dont-cycle)
+  "Complete the minibuffer string between START and END to an exact match.
+Repeated uses step through the possible completions, unless
+prevented to do so by DONT-CYCLE."
   (interactive)
   (setq minibuffer-scroll-window nil)
   ;; FIXME: Need to deal with the extra-size issue here as well.
   ;; FIXME: ~/src/emacs/t<M-TAB>/lisp/minibuffer.el completes to
   ;; ~/src/emacs/trunk/ and throws away lisp/minibuffer.el.
+  ;; FIXME: shouldn't we cycle _before_ instead of after forcing??
   (let* ((start (copy-marker (or start (minibuffer-prompt-end))))
          (end (or end (point-max)))
          ;; (md (completion--field-metadata start))
          (all (completion-all-sorted-completions start end))
-         (base (+ start (or (cdr (last all)) 0))))
+         (base (+ start (or (cdr (last all)) 0)))
+         last second-last)
     (cond
      ((not (consp all))
-        (completion--message
+      (completion--message
        (if all "No more completions" "No completions")))
      ((not (consp (cdr all)))
       (let ((done (equal (car all) (buffer-substring-no-properties base end))))
@@ -1287,36 +1290,48 @@ minibuffer-force-complete
         (completion--done (buffer-substring-no-properties start (point))
                           'finished (when done "Sole completion"))))
      (t
+      (setq second-last (last all 2)
+            last        (cdr second-last))
+      ;; If we happened to be already cycling, we must undo the
+      ;; effects of the last rotation (get out yer' pen and paper to
+      ;; get the cons cell math).
+      (when (and dont-cycle completion-cycling)
+        (let ((lastcdr (cddr second-last)))
+          (setcdr (cdr second-last) all)
+          (setq all (cdr second-last))
+          (setcdr second-last lastcdr)
+          (setq last second-last)))
       (completion--replace base end (car all))
       (setq end (+ base (length (car all))))
       (completion--done (buffer-substring-no-properties start (point)) 'sole)
-      ;; Set cycling after modifying the buffer since the flush hook resets it.
-      (setq completion-cycling t)
-      (setq this-command 'completion-at-point) ;For completion-in-region.
-      ;; If completing file names, (car all) may be a directory, so we'd now
-      ;; have a new set of possible completions and might want to reset
-      ;; completion-all-sorted-completions to nil, but we prefer not to,
-      ;; so that repeated calls minibuffer-force-complete still cycle
-      ;; through the previous possible completions.
-      (let ((last (last all)))
+      (unless dont-cycle
+        ;; Set cycling after modifying the buffer since the flush hook resets it.
+        (setq completion-cycling t)
+        (setq this-command 'completion-at-point) ;For completion-in-region.
+        ;; Rotate the completions collected earlier and cache them.  If
+        ;; completing file names, (car all) may be a directory, so we'd
+        ;; now have a new set of possible completions and might want to
+        ;; reset completion-all-sorted-completions to nil, but we prefer
+        ;; not to, so that repeated calls minibuffer-force-complete
+        ;; still cycle through the previous possible completions.
         (setcdr last (cons (car all) (cdr last)))
-        (completion--cache-all-sorted-completions start end (cdr all)))
-      ;; Make sure repeated uses cycle, even though completion--done might
-      ;; have added a space or something that moved us outside of the field.
-      ;; (bug#12221).
-      (let* ((table minibuffer-completion-table)
-             (pred minibuffer-completion-predicate)
-             (extra-prop completion-extra-properties)
-             (cmd
-              (lambda () "Cycle through the possible completions."
-                (interactive)
-                (let ((completion-extra-properties extra-prop))
-                  (completion-in-region start (point) table pred)))))
-        (set-transient-map
-         (let ((map (make-sparse-keymap)))
-           (define-key map [remap completion-at-point] cmd)
-           (define-key map (vector last-command-event) cmd)
-           map)))))))
+        (completion--cache-all-sorted-completions start end (cdr all))
+        ;; Make sure repeated uses cycle, even though completion--done
+        ;; might have added a space or something that moved us outside
+        ;; of the field.  (bug#12221).
+        (let* ((table minibuffer-completion-table)
+               (pred minibuffer-completion-predicate)
+               (extra-prop completion-extra-properties)
+               (cmd
+                (lambda () "Cycle through the possible completions."
+                  (interactive)
+                  (let ((completion-extra-properties extra-prop))
+                    (completion-in-region start (point) table pred)))))
+          (set-transient-map
+           (let ((map (make-sparse-keymap)))
+             (define-key map [remap completion-at-point] cmd)
+             (define-key map (vector last-command-event) cmd)
+             map))))))))
 
 (defvar minibuffer-confirm-exit-commands
   '(completion-at-point minibuffer-complete
-- 
2.19.2


Added tag(s) patch. Request was from João Távora <joaotavora <at> gmail.com> to control <at> debbugs.gnu.org. (Thu, 17 Jan 2019 14:00:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Thu, 17 Jan 2019 14:58:01 GMT) Full text and rfc822 format available.

Message #10 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: João Távora <joaotavora <at> gmail.com>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: 27.0.50; minibuffer-force-complete-and-exit mostly broken
Date: Thu, 17 Jan 2019 09:57:10 -0500
> @@ -1257,29 +1257,32 @@ completion-all-sorted-completions
>  (defun minibuffer-force-complete-and-exit ()
>    "Complete the minibuffer with first of the matches and exit."
>    (interactive)
> -  (minibuffer-force-complete)
> +  (minibuffer-force-complete nil nil t)
>    (completion--complete-and-exit
>     (minibuffer-prompt-end) (point-max) #'exit-minibuffer
>     ;; If the previous completion completed to an element which fails
>     ;; test-completion, then we shouldn't exit, but that should be rare.
>     (lambda () (minibuffer-message "Incomplete"))))

Wouldn't it be simpler to change minibuffer-force-complete-and-exit so
it checks test-completion before calling minibuffer-force-complete?


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Thu, 17 Jan 2019 15:05:04 GMT) Full text and rfc822 format available.

Message #13 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: 27.0.50; minibuffer-force-complete-and-exit mostly broken
Date: Thu, 17 Jan 2019 15:03:43 +0000
On Thu, Jan 17, 2019 at 2:57 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> > @@ -1257,29 +1257,32 @@ completion-all-sorted-completions
> >  (defun minibuffer-force-complete-and-exit ()
> >    "Complete the minibuffer with first of the matches and exit."
> >    (interactive)
> > -  (minibuffer-force-complete)
> > +  (minibuffer-force-complete nil nil t)
> >    (completion--complete-and-exit
> >     (minibuffer-prompt-end) (point-max) #'exit-minibuffer
> >     ;; If the previous completion completed to an element which fails
> >     ;; test-completion, then we shouldn't exit, but that should be rare.
> >     (lambda () (minibuffer-message "Incomplete"))))
>
> Wouldn't it be simpler to change minibuffer-force-complete-and-exit so
> it checks test-completion before calling minibuffer-force-complete?

Makes sense.  As I explained elsewhere, I am a total completion API
newbie.  I'm always unsure what to pass to these functions and the
dark logic they engage in.  On the contrary, my change is based on
cons cells, which I still understand :-)

But your suggestion makes perfect sense and I'd be very
thankful if you could do it yourself (if it is as trivial as it sounds).

João Távora




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Fri, 18 Jan 2019 12:14:02 GMT) Full text and rfc822 format available.

Message #16 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 12:13:06 +0000
I'm going to change my stance on this, but just a little.
I trid your approach, passing minibuffer-completion-table
and minibuffer-completion-predicate and some "reasonable"
string to try-completion.

It seems to work most of the times, but I could swear it is
is doing mischief in some situations (can't tell which tho).

Other arguments against it is that it shouldn't be necessary
to query the table again by this point, which is potentially
slow and could have side effects depending on the table.

In other words, there is some arguably poorly chosen cons
juggling going on in minibuffer-force-complete.  My fix adds
to that, but at least it stays within the same idiom.

So if you don't mind I'd fix this rather serious bug with my
safer fix, and later we can use your try-completion approach.

João



On Thu, Jan 17, 2019 at 3:11 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> On Thu, Jan 17, 2019 at 2:57 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> >
> > > @@ -1257,29 +1257,32 @@ completion-all-sorted-completions
> > >  (defun minibuffer-force-complete-and-exit ()
> > >    "Complete the minibuffer with first of the matches and exit."
> > >    (interactive)
> > > -  (minibuffer-force-complete)
> > > +  (minibuffer-force-complete nil nil t)
> > >    (completion--complete-and-exit
> > >     (minibuffer-prompt-end) (point-max) #'exit-minibuffer
> > >     ;; If the previous completion completed to an element which fails
> > >     ;; test-completion, then we shouldn't exit, but that should be rare.
> > >     (lambda () (minibuffer-message "Incomplete"))))
> >
> > Wouldn't it be simpler to change minibuffer-force-complete-and-exit so
> > it checks test-completion before calling minibuffer-force-complete?
>
> Makes sense.  As I explained elsewhere, I am a total completion API
> newbie.  I'm always unsure what to pass to these functions and the
> dark logic they engage in.  On the contrary, my change is based on
> cons cells, which I still understand :-)
>
> But your suggestion makes perfect sense and I'd be very
> thankful if you could do it yourself (if it is as trivial as it sounds).
>
> João Távora
>
>
>


--
João Távora




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Fri, 18 Jan 2019 13:10:02 GMT) Full text and rfc822 format available.

Message #19 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: João Távora <joaotavora <at> gmail.com>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 08:09:15 -0500
> It seems to work most of the times, but I could swear it is
> is doing mischief in some situations (can't tell which tho).

I can believe it.

> Other arguments against it is that it shouldn't be necessary
> to query the table again by this point, which is potentially
> slow and could have side effects depending on the table.

test-completion should be pretty fast (and it's called by the subsequent
completion--complete-and-exit so no matter how slow, adding a call
cannot slowdown the command by more than a factor 2x).

> In other words, there is some arguably poorly chosen cons
> juggling going on in minibuffer-force-complete.  My fix adds
> to that, but at least it stays within the same idiom.

I must admit that I find your fix a bit ugly (adding an ad-hoc extra
arg, then undoing cycling depending on (and dont-cycle
completion-cycling), ...).

Taking another look at the problem, I believe the change below is the
better option.  Any objection?


        Stefan


diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index db8d4ba5ce..879ad9a8f7 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1378,7 +1381,8 @@ completion-all-sorted-completions
 (defun minibuffer-force-complete-and-exit ()
   "Complete the minibuffer with first of the matches and exit."
   (interactive)
-  (minibuffer-force-complete)
+  (unless completion-cycling
+    (minibuffer-force-complete))
   (completion--complete-and-exit
    (minibuffer-prompt-end) (point-max) #'exit-minibuffer
    ;; If the previous completion completed to an element which fails




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Fri, 18 Jan 2019 13:29:01 GMT) Full text and rfc822 format available.

Message #22 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 13:28:39 +0000
On Fri, Jan 18, 2019 at 1:09 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
> > It seems to work most of the times, but I could swear it is
> > is doing mischief in some situations (can't tell which tho).
>
> I can believe it.

I'll try to pin it down later.

> test-completion should be pretty fast (and it's called by the subsequent
> completion--complete-and-exit so no matter how slow, adding a call
> cannot slowdown the command by more than a factor 2x)

OK, though not particularly reassuring :-)

> > In other words, there is some arguably poorly chosen cons
> > juggling going on in minibuffer-force-complete.  My fix adds
> > to that, but at least it stays within the same idiom.
>
> I must admit that I find your fix a bit ugly (adding an ad-hoc extra
> arg, then undoing cycling depending on (and dont-cycle
> completion-cycling), ...).

Oh yes, no problem admitting that.  It's hideous!  But small
enough to yield to local analysis and be easy to analyse
to be safe.

> Taking another look at the problem, I believe the change below is the
> better option.  Any objection?

Yes, it elegantly fixes this bug, but complicates #34077.
OTOH #34077 could be solved by your try-completion
thing.  But as I said I think that is unstable (or at least has
much more potential to be unstablel).

commit 74ff04e5cd271a238cb9bd966cdbcf9804d49386
Author: João Távora <joaotavora <at> gmail.com>
Date:   Tue Jan 15 16:50:40 2019 +0000

  Avoid broken C-M-i cycling in icomplete-mode (bug#34077)

  If there is only one propective candidate and it happens to be a
  directory then (1) C-M-i causes the prospects to be updated to the subfiles of
  the completed directory, otherwise (2) the prospects are merely rotated.

  It is very hard to tell if (1) or (2) happened because the rotated
  prospects may look identical to the updated prospects.  Therefore, in
  icomplete-mode, it is preferable to do (1) always, to avoid
  iconsistencies with the presentation of prospects in this mode.  There
  are already facilities in place to rotate the prospects list.

  * lisp/icomplete.el (icomplete-minibuffer-map): Bind C-M-i to
  icomplete-force-complete.
  (icomplete-force-complete): New command.

The current fix I have for it is

(defvar icomplete-minibuffer-map
   (let ((map (make-sparse-keymap)))
-    (define-key map [?\M-\t] 'minibuffer-force-complete)
+    (define-key map [?\M-\t] 'icomplete-force-complete)
     (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-force-complete ()
+  "Complete the minibuffer.
+Like `minibuffer-force-complete', but don't cycle."
+  (interactive)
+  ;; FIXME: it _could_ make sense to cycle in certain situations, by
+  ;; analyzing the current thing and the thing to cycle to for
+  ;; instance.  Unfortunately that can't be done until a _very nasty
+  ;; hack_ in `minibuffer-force-complete' is removed.  That hack uses
+  ;; transient maps and prevents two consecutive calls to
+  ;; `icomplete-force-complete'.
+  (minibuffer-force-complete nil nil t))

João Távora




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Fri, 18 Jan 2019 18:01:01 GMT) Full text and rfc822 format available.

Message #25 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: João Távora <joaotavora <at> gmail.com>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 13:00:35 -0500
> Yes, it elegantly fixes this bug, but complicates #34077.

I don't see why.  Can you explain?


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Fri, 18 Jan 2019 22:19:02 GMT) Full text and rfc822 format available.

Message #28 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 22:18:22 +0000
Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>> Yes, it elegantly fixes this bug, but complicates #34077.
>
> I don't see why.  Can you explain?

If I do that, then C-M-i will still cycle/rotate after completing say,
'/emacs/sr' to '/emacs/src', which means that
completion-all-sorted-completions, used by the prospects, listing is
still forcibly cached by minibuffer-force-complete.

The reason it shouldn't be, IMO, is that the result is I will now see
'{src | libsrc}' as candidates, instead of seeing the files under
emacs/src/.  The problem is really that now I can't know if those are
actual dirs emacs/src/src emacs/src/libsrc.

You could argue that's just my personal preference and the cycling still
makes sense, so I put this new behaviour is a separate command
icomplete-force-complete as a compromise.  But with your technique I
can't implement it properly.

João




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Sat, 19 Jan 2019 02:32:02 GMT) Full text and rfc822 format available.

Message #31 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: João Távora <joaotavora <at> gmail.com>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Fri, 18 Jan 2019 21:31:55 -0500
>>> Yes, it elegantly fixes this bug, but complicates #34077.
>> I don't see why.  Can you explain?
> If I do that, then C-M-i will still cycle/rotate after completing say,
> '/emacs/sr' to '/emacs/src', which means that
> completion-all-sorted-completions, used by the prospects, listing is
> still forcibly cached by minibuffer-force-complete.

Right, but the patch doesn't affect minibuffer-force-complete at all, so
it doesn't make the problem any harder, does it?


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Wed, 23 Jan 2019 15:50:01 GMT) Full text and rfc822 format available.

Message #34 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Wed, 23 Jan 2019 15:49:00 +0000
On Sat, Jan 19, 2019 at 2:31 AM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> >>> Yes, it elegantly fixes this bug, but complicates #34077.
> >> I don't see why.  Can you explain?
> > If I do that, then C-M-i will still cycle/rotate after completing say,
> > '/emacs/sr' to '/emacs/src', which means that
> > completion-all-sorted-completions, used by the prospects, listing is
> > still forcibly cached by minibuffer-force-complete.
>
> Right, but the patch doesn't affect minibuffer-force-complete at all, so
> it doesn't make the problem any harder, does it?

No but to solve the other problem, I still need the DONT-CYCLE hack.
What I should have said is "this doesn't solve my other problem".
Indeed it doesn't make it any harder.

But what about this which solves both problems bug#34077
and bug#34116?

diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 6d77c0649a..562da1a71d 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -145,7 +145,7 @@ icomplete-post-command-hook

 (defvar icomplete-minibuffer-map
   (let ((map (make-sparse-keymap)))
-    (define-key map [?\M-\t] 'minibuffer-force-complete)
+    (define-key map [?\M-\t] 'icomplete-force-complete)
     (define-key map [?\C-j]  'icomplete-force-complete-and-exit)
     (define-key map [?\C-.]  'icomplete-forward-completions)
     (define-key map [?\C-,]  'icomplete-backward-completions)
@@ -162,6 +162,21 @@ icomplete-force-complete-and-exit
       (minibuffer-force-complete-and-exit)
     (minibuffer-complete-and-exit)))

+(defun icomplete-force-complete ()
+  "Complete the icomplete minibuffer."
+  (interactive)
+  (let ((retval (minibuffer-force-complete)))
+    ;; FIXME: What's this you ask?  Do deal with a cycling corner
+    ;; case, `minibuffer-force-complete' will replace the keybinding
+    ;; that `icomplete-force-complete' was called with, but at least
+    ;; returns a function which we can call to disable that, since
+    ;; we're not interested in cycling at all here (bug#34077).
+    (when (and completion-cycling (functionp retval)) (funcall retval)))
+  ;; Again, since we're not interested in cycling, we want prospects
+  ;; to be recalcualted, and not based on cached, rotated completions.
+  (setq completion-cycling nil)
+  (setq completion-all-sorted-completions nil))
+
 (defun icomplete-forward-completions ()
   "Step forward completions by one entry.
 Second entry becomes the first and can be selected with
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5760a2e49d..b06f13251f 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1257,7 +1257,12 @@ completion-all-sorted-completions
 (defun minibuffer-force-complete-and-exit ()
   "Complete the minibuffer with first of the matches and exit."
   (interactive)
-  (minibuffer-force-complete)
+  ;; If `completion-cycling' is t, then surely a
+  ;; `minibuffer-force-complete' has already executed.  This is not
+  ;; for speed: the extra rotation caused by the second unnecessary call
+  ;; would mess up the final result value (bug#34116).
+  (unless completion-cycling
+    (minibuffer-force-complete))
   (completion--complete-and-exit
    (minibuffer-prompt-end) (point-max) #'exit-minibuffer
    ;; If the previous completion completed to an element which fails




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Wed, 23 Jan 2019 16:10:01 GMT) Full text and rfc822 format available.

Message #37 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: João Távora <joaotavora <at> gmail.com>
Cc: 34116 <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Wed, 23 Jan 2019 11:09:45 -0500
> But what about this which solves both problems bug#34077
> and bug#34116?

LGTM,


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#34116; Package emacs. (Wed, 23 Jan 2019 16:37:02 GMT) Full text and rfc822 format available.

Message #40 received at 34116 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34116 <at> debbugs.gnu.org, 34116-done <at> debbugs.gnu.org,
 bug#34077 <34077 <at> debbugs.gnu.org>, 34077-done <at> debbugs.gnu.org
Subject: Re: bug#34116: 27.0.50;
 minibuffer-force-complete-and-exit mostly broken
Date: Wed, 23 Jan 2019 16:36:07 +0000
On Wed, Jan 23, 2019 at 4:09 PM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> > But what about this which solves both problems bug#34077
> > and bug#34116?
>
> LGTM,
>         Stefan

Great!

commit b9add0a5a7eddcf80a189c478b39a5cb7a12befe (HEAD -&gt; master,
origin/master, origin/HEAD)
Author: João Távora
Date:   Wed Jan 23 16:30:41 2019 +0000

    Force completion in icomplete with C-M-i, but don't cycle
(bug#34077)

    Cycling after forcing a completion with C-M-i in icomplete can be
    confusing, as it leaves rotated prospects in the minibuffer.  In
C-x
    C-f, for example it is very difficult to understand if the
prospects
    refer to subdirectories of the directory being completed to, which
    happens naturally when the completion is unique; or if they are a
    cycled version of prospects that match the new completion pattern,
in
    case the completion happens to still match other items.

    To resolve this confusion, never cycle with C-M-i in icomplete:
    non-ambiguous cycling can be achieved with C-. and C-,

    The former behaviour can still be restored with:

    (define-key icomplete-minibuffer-map (kbd "C-M-i")
'minibuffer-force-complete)

    * lisp/icomplete.el (icomplete-force-complete): New command.
    (icomplete-minibuffer-map): Bind C-M-i to
icomplete-force-complete.

commit 210e592e55ade154c8d58bd467711fb69368f6cb
Author: João Távora
Date:   Wed Jan 23 16:17:03 2019 +0000

    Avoid cycling in minibuffer-force-complete-and-exit (bug#34116)

    * lisp/minibuffer.el (minibuffer-force-complete-and-exit): Check
    completion-cycling before minibuffer-force-complete.




Reply sent to João Távora <joaotavora <at> gmail.com>:
You have taken responsibility. (Wed, 23 Jan 2019 16:37:03 GMT) Full text and rfc822 format available.

Notification sent to João Távora <joaotavora <at> gmail.com>:
bug acknowledged by developer. (Wed, 23 Jan 2019 16:37:04 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 21 Feb 2019 12:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 5 years and 65 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.