unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil
@ 2010-10-22 15:03 Fran Litterio
  2010-10-28  2:23 ` Stefan Monnier
  0 siblings, 1 reply; 3+ messages in thread
From: Fran Litterio @ 2010-10-22 15:03 UTC (permalink / raw)
  To: 7266

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)





^ permalink raw reply	[flat|nested] 3+ messages in thread

* bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil
  2010-10-22 15:03 bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil Fran Litterio
@ 2010-10-28  2:23 ` Stefan Monnier
  2010-10-28 21:40   ` Fran Litterio
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Monnier @ 2010-10-28  2:23 UTC (permalink / raw)
  To: Fran Litterio; +Cc: 7266

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

        





^ permalink raw reply	[flat|nested] 3+ messages in thread

* bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil
  2010-10-28  2:23 ` Stefan Monnier
@ 2010-10-28 21:40   ` Fran Litterio
  0 siblings, 0 replies; 3+ messages in thread
From: Fran Litterio @ 2010-10-28 21:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 7266

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





^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-28 21:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-22 15:03 bug#7266: Patch to fix minibuffer-complete when icomplete-mode is on and completion-cycle-threshold is nil Fran Litterio
2010-10-28  2:23 ` Stefan Monnier
2010-10-28 21:40   ` Fran Litterio

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).