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

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 7266 in the body.
You can then email your comments to 7266 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: Fran Litterio <flitterio <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Patch to fix minibuffer-complete when icomplete-mode is on and
	completion-cycle-threshold is nil
Date: Fri, 22 Oct 2010 11:03:44 -0400

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)

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Fran Litterio <flitterio <at> gmail.com>
Cc: 7266 <at> debbugs.gnu.org
Subject: Re: Patch to fix minibuffer-complete when icomplete-mode is on and
	completion-cycle-threshold is nil
Date: Wed, 27 Oct 2010 22:23:15 -0400

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

        

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

From: Fran Litterio <flitterio <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 7266 <at> debbugs.gnu.org
Subject: Re: Patch to fix minibuffer-complete when icomplete-mode is on and
	completion-cycle-threshold is nil
Date: Thu, 28 Oct 2010 17:40:43 -0400

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

Previous Next

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