GNU bug report logs - #34077
27.0.50; icomplete, substring completion and C-x C-f misbehaving

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 34077 in the body.
You can then email your comments to 34077 AT debbugs.gnu.org in the normal way.

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

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; icomplete, substring completion and C-x C-f misbehaving
Date: Mon, 14 Jan 2019 16:32:42 +0000

Hi maintainers,

I'm using substring completion, icomplete-mode and C-x C-f.  Navigating
deeper in directories using successive C-M-i's sometimes fails:
 
  Emacs -Q
  M-: (add-to-list 'completion-styles 'substring)
  M-x icomplete-mode
  C-x C-f p a t h / t o / e m a c s /
  s r c C-M-i
 
Expected to see the same as if I had typed "s r c /", instead I see
"{lib-src | src }".

This doesn't happen all the time: in this situation it seems to have
become confused by the fact that "lib-src" contains the substring "src"

João

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

From: João Távora <joaotavora <at> gmail.com>
To: 34077 <at> debbugs.gnu.org
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#34077: 27.0.50;
 icomplete, substring completion and C-x C-f misbehaving
Date: Tue, 15 Jan 2019 17:03:20 +0000

[Message part 1 (text/plain, inline)]

On Mon, Jan 14, 2019 at 4:43 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> Hi maintainers,
>
> I'm using substring completion, icomplete-mode and C-x C-f.  Navigating
> deeper in directories using successive C-M-i's sometimes fails:
>
>   Emacs -Q
>   M-: (add-to-list 'completion-styles 'substring)
>   M-x icomplete-mode
>   C-x C-f p a t h / t o / e m a c s /
>   s r c C-M-i
>
> Expected to see the same as if I had typed "s r c /", instead I see
> "{lib-src | src }".

Discovered that this is by design in normal non-icomplete.el
conditions, i.e. minibuffer-force-completion is supposed to cycle.

However in icomplete-mode it doesn't make sense because
its "prospects list" will show what appears to be subfiles of the
completed directory.  Crucially, sometimes they *will* be subfiles
of the completed directory (and that depends on whether the user's
pattern matched one or more before the forced completion, which
cannot be known anymore).

Attach a patch that fixes this for icomplete.el only.

-- 
João Távora

[0001-Repeated-C-M-i-doesn-t-cycle-directories-in-icomplet.patch (application/octet-stream, attachment)]

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

From: João Távora <joaotavora <at> gmail.com>
To: bug#34077 <34077 <at> debbugs.gnu.org>
Subject: Re: Status: 27.0.50;
 icomplete, substring completion and C-x C-f misbehaving
Date: Thu, 17 Jan 2019 14:02:39 +0000

[Message part 1 (text/plain, inline)]

tag 34077 patch

Here's a simpler patch set to fix this:

[0001-Fix-nasty-cycling-on-minibuffer-force-complete-and-e.patch (text/x-patch, inline)]

From cc389a92a51dfab1707246d77d03c387b02f312d 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 1/5] Fix nasty cycling on minibuffer-force-complete-and-exit
 (bug#34116)

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

[0005-Avoid-broken-C-M-i-cycling-in-icomplete-mode-bug-340.patch (text/x-patch, inline)]

From f5538c8febc5f28ef08e30fe98b8b91c8fe1e525 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Tue, 15 Jan 2019 16:50:40 +0000
Subject: [PATCH 5/5] 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.
---
 lisp/icomplete.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 6d77c0649a..80357af4ad 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -145,13 +145,25 @@ 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)
     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))
+
 (defun icomplete-force-complete-and-exit ()
   "Complete the minibuffer and exit.
 Use the first of the matches if there are any displayed, and use
-- 
2.19.2

Message #14 received at 34077 <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.

Previous Next

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