bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil

[Stefan Monnier <monnier@iro.umontreal.ca>]

> 2. Type: ESC ESC : (icomplete-mode 1) RET C-h v mini TAB TAB

> Notice that minibuffer-complete (which is bound to TAB at this point)
> is cycling through the completion choices instead of popping up a
> window to display the completion choices. This bug is caused code in
> lisp/minibuffer.el that does not check that the value of
> completion-cycle-threshold is not nil before deciding to cycle
> completions. This patch (to the Bazaar sources) fixes the bug:

Thanks very much.  Indeed, there's a problem here, but...

> @@ -607,7 +607,8 @@
>                     (completion-all-sorted-completions))))
>              (setq completion-all-sorted-completions nil)
>              (cond
> -             ((and (not (ignore-errors
> +             ((and completion-cycle-threshold  ;; Never cycle if
> completion-cycle-threshold is nil.
> +                  (not (ignore-errors
>                            ;; This signal an (intended) error if comps is too
>                            ;; short or if completion-cycle-threshold is t.
>                            (consp (nthcdr completion-cycle-threshold comps))))

Hmm... AFAICT if completion-cycle-threshold is nil, then comps will also
be nil, so this change should not make any difference.

> @@ -664,7 +665,8 @@
>             (scroll-other-window))
>          nil)))
>     ;; If we're cycling, keep on cycling.
> -   (completion-all-sorted-completions
> +   ((and completion-cycle-threshold    ;; Never cycle if
> completion-cycle-threshold is nil.
> +        completion-all-sorted-completions)
>      (minibuffer-force-complete)
>      t)
>     (t (case (completion--do-completion)

It goes a bit further than that: even if completion-cycle-threshold is
non-nil and completion-all-sorted-completions is set, it may still be
wrong to call minibuffer-force-complete since
completion-all-sorted-completions may only be set because of icomplete
rather than because we're cycling (e.g. the completion list may be
larger than the threshold).

So I've installed the patch below instead.  Please confirm that it fixes
the problem.


        Stefan
        
        
=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2010-10-19 11:44:07 +0000
+++ lisp/minibuffer.el	2010-10-28 02:16:06 +0000
@@ -526,6 +526,10 @@
           (const :tag "Always cycle" t)
           (integer :tag "Threshold")))
 
+(defvar completion-all-sorted-completions nil)
+(make-variable-buffer-local 'completion-all-sorted-completions)
+(defvar completion-cycling nil)
+
 (defun completion--do-completion (&optional try-completion-function)
   "Do the completion and return a summary of what happened.
 M = completion was performed, the text was Modified.
@@ -605,14 +609,13 @@
                                          ""))
                                    comp-pos)))
                    (completion-all-sorted-completions))))
-            (setq completion-all-sorted-completions nil)
+            (completion--flush-all-sorted-completions)
             (cond
-             ((and (not (ignore-errors
+             ((and (consp (cdr comps)) ;; There's something to cycle.
+                   (not (ignore-errors
                           ;; This signal an (intended) error if comps is too
                           ;; short or if completion-cycle-threshold is t.
-                          (consp (nthcdr completion-cycle-threshold comps))))
-                   ;; More than 1, so there's something to cycle.
-                   (consp (cdr comps)))
+                          (consp (nthcdr completion-cycle-threshold comps)))))
               ;; Fewer than completion-cycle-threshold remaining
               ;; completions: let's cycle.
               (setq completed t exact t)
@@ -648,7 +651,7 @@
   ;; If the previous command was not this,
   ;; mark the completion buffer obsolete.
   (unless (eq this-command last-command)
-    (setq completion-all-sorted-completions nil)
+    (completion--flush-all-sorted-completions)
     (setq minibuffer-scroll-window nil))
 
   (cond
@@ -664,7 +667,7 @@
 	    (scroll-other-window))
         nil)))
    ;; If we're cycling, keep on cycling.
-   (completion-all-sorted-completions
+   ((and completion-cycling completion-all-sorted-completions)
     (minibuffer-force-complete)
     t)
    (t (case (completion--do-completion)
@@ -675,10 +678,8 @@
                t)
         (t     t)))))
 
-(defvar completion-all-sorted-completions nil)
-(make-variable-buffer-local 'completion-all-sorted-completions)
-
 (defun completion--flush-all-sorted-completions (&rest ignore)
+  (setq completion-cycling nil)
   (setq completion-all-sorted-completions nil))
 
 (defun completion-all-sorted-completions ()
@@ -720,6 +721,7 @@
          (all (completion-all-sorted-completions)))
     (if (not (consp all))
         (minibuffer-message (if all "No more completions" "No completions"))
+      (setq completion-cycling t)
       (goto-char end)
       (insert (car all))
       (delete-region (+ start (cdr (last all))) end)