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

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

[Fran Litterio]

To reproduce the bug do this:

1. Launch "emacs -q".

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:

--- lisp/minibuffer.el.orig     2010-10-21 17:55:46.380857900 -0400
+++ lisp/minibuffer.el  2010-10-22 10:29:29.188417500 -0400
@@ -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))))
@@ -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)

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

[Stefan Monnier]

> 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)

        

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

[Fran Litterio]

On Wed, Oct 27, 2010 at 10:23 PM, Stefan Monnier wrote:
>> 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.

> Thanks very much.  Indeed, there's a problem here, but...
[...]
> Hmm... AFAICT if completion-cycle-threshold is nil, then comps will also
> be nil, so this change should not make any difference.

Oddly, my change _did_ fix the problem for me.

> 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.

I've verified that your patch does indeed fix the problem.

Thanks, Stefan!
--
Fran