bug#34077: 27.0.50; icomplete, substring completion and C-x C-f misbehaving

["João Távora" <joaotavora@gmail.com>]

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

On Mon, Jan 14, 2019 at 4:43 PM João Távora <joaotavora@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

[-- Attachment #2: 0001-Repeated-C-M-i-doesn-t-cycle-directories-in-icomplet.patch --]
[-- Type: application/octet-stream, Size: 7254 bytes --]

From 837833c66c638b3d863f142ff37e563b27a3672e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Tue, 15 Jan 2019 16:50:40 +0000
Subject: [PATCH] Repeated C-M-i doesn't cycle directories in icomplete-mode

Depending on wether there is one or more directories matching a
certain pattern, when C-M-i completes to a directory name:

1. sometimes prospects are updated to the subfiles of the completed directory
2. sometimes they stay on the parent dir, giving the impression that the
   directory has a subdirectory with the same name.

In icomplete-mode, it is preferable to do 1 always, to avoid
iconsistencies with the presentation of prospects in this mode.

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

* lisp/minibuffer.el (minibuffer-force-complete): Accept new arg
DONT-CYCLE-DIRS.
---
 lisp/icomplete.el  | 10 +++++--
 lisp/minibuffer.el | 69 +++++++++++++++++++++++++---------------------
 2 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index c2cf4fab58..d6f60a8820 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -145,13 +145,19 @@ 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)
+  (minibuffer-force-complete nil nil 'dont-cycle))
+
 (defun icomplete-force-complete-and-exit ()
   "Complete the minibuffer and exit.
 Use the first of the matches if there are any displayed, and use
@@ -370,7 +376,7 @@ icomplete-completions
 matches exist."
   (let* ((ignored-extension-re
           (and completion-ignored-extensions
-               (concat "\\(?:\\`\\.\\.?/\\|"
+               (concat "\\(?:\\`\\.\\./\\|"
                        (regexp-opt completion-ignored-extensions)
                        "\\)\\'")))
          (minibuffer-completion-table candidates)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 578fd9ab63..c4ed2ef004 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1272,9 +1272,11 @@ minibuffer-force-complete-and-exit
    ;; test-completion, then we shouldn't exit, but that should be rare.
    (lambda () (minibuffer-message "Incomplete"))))
 
-(defun minibuffer-force-complete (&optional start end)
+(defun minibuffer-force-complete (&optional start end dont-cycle-dirs)
   "Complete the minibuffer to an exact match.
-Repeated uses step through the possible completions."
+Repeated uses step through the possible completions, unless
+DONT-CYCLE-DIRS is non-nil and minibuffer completed to a
+directory name."
   (interactive)
   (setq minibuffer-scroll-window nil)
   ;; FIXME: Need to deal with the extra-size issue here as well.
@@ -1284,7 +1286,8 @@ minibuffer-force-complete
          (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)))
+         sole)
     (cond
      ((not (consp all))
         (completion--message
@@ -1297,34 +1300,38 @@ minibuffer-force-complete
      (t
       (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)))
-        (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--done
+       (setq sole (buffer-substring-no-properties start (point))) 'sole)
+      (unless (and dont-cycle-dirs
+                   minibuffer-completing-file-name
+                   (file-directory-p 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)))
+          (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))))))))
 
 (defvar minibuffer-confirm-exit-commands
   '(completion-at-point minibuffer-complete
-- 
2.19.2