GNU bug report logs - #10062
24.0.91; completions-first-difference

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 10062 in the body.
You can then email your comments to 10062 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: Leo <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.91; completions-first-difference
Date: Wed, 16 Nov 2011 17:45:23 +0800

[Message part 1 (text/plain, inline)]

1. Emacs -q
2. (setq completion-cycle-threshold 4)
3. In the *scratch* buffer type "(push" without the double quotes
4. Type M-Tab a few times

You should see something like in the screenshot:

[emacs-completion-face.png (image/png, attachment)]

[Message part 3 (text/plain, inline)]

BTW, I think the face property completions-first-difference should not
be rear-sticky. When that char is the last in the completion anything I
type after inherits it as seen in this screenshot:

[emacs-slime-repl.png (image/png, attachment)]

[Message part 5 (text/plain, inline)]

In GNU Emacs 24.0.91.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2011-11-09
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
configured using `configure  'CFLAGS=-g' '--prefix=/usr/local/unix/emacs' '--with-wide-int' '--with-x-toolkit=lucid' '--without-gsettings''

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

From: Leo <sdl.web <at> gmail.com>
To: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Fri, 18 Nov 2011 00:25:15 +0800

On 2011-11-16 17:45 +0800, Leo wrote:
> 1. Emacs -q
> 2. (setq completion-cycle-threshold 4)
> 3. In the *scratch* buffer type "(push" without the double quotes
> 4. Type M-Tab a few times

Any objection to the following patch:

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2011-10-17 16:30:02 +0000
+++ lisp/minibuffer.el	2011-11-17 16:22:45 +0000
@@ -1203,9 +1203,10 @@
                                'font-lock-face 'completions-common-part
                                str)
             (if (> (length str) com-str-len)
-                (put-text-property com-str-len (1+ com-str-len)
-                                   'font-lock-face 'completions-first-difference
-                                   str)))
+                (add-text-properties com-str-len (1+ com-str-len)
+				     '(font-lock-face completions-first-difference
+						      rear-nonsticky t)
+				     str)))
           elem)
         completions)
        base-size))))


Leo

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo <sdl.web <at> gmail.com>
Cc: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Thu, 17 Nov 2011 20:47:34 -0500

>> 1. Emacs -q
>> 2. (setq completion-cycle-threshold 4)
>> 3. In the *scratch* buffer type "(push" without the double quotes
>> 4. Type M-Tab a few times

> Any objection to the following patch:

Yup: the bug is not in the rear-stickiness but in the mere presence of
this face.  I.e. the fix is to strip these faces when used for
completion (but of course keep them when used for *Completions* display).


        Stefan

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

From: Leo <sdl.web <at> gmail.com>
To: 10062 <at> debbugs.gnu.org
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Thu, 01 Dec 2011 12:20:54 +0800

> Yup: the bug is not in the rear-stickiness but in the mere presence of
> this face.  I.e. the fix is to strip these faces when used for
> completion (but of course keep them when used for *Completions*
> display).

Sorry for the delay.

It is kind of nice to indicate the first different char when used with
cycle completion. WDYT?

Leo

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo <sdl.web <at> gmail.com>
Cc: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Thu, 01 Dec 2011 10:57:05 -0500

>> Yup: the bug is not in the rear-stickiness but in the mere presence of
>> this face.  I.e. the fix is to strip these faces when used for
>> completion (but of course keep them when used for *Completions*
>> display).
> Sorry for the delay.
> It is kind of nice to indicate the first different char when used with
> cycle completion. WDYT?

I guess it could make sense to keep the face on the inserted text while
you're still cycling, but it should be removed afterwards.  And since
the transition between "cycling" and "not cycling" is not explicit,
you'd then need/want to remove the face from something like
a pre-command-hook.


        Stefan

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

From: Leo <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Mon, 16 Jan 2012 17:18:54 +0800

[Message part 1 (text/plain, inline)]

On 2011-11-18 09:47 +0800, Stefan Monnier wrote:
[snipped 7 lines]
> Yup: the bug is not in the rear-stickiness but in the mere presence of
> this face.  I.e. the fix is to strip these faces when used for
> completion (but of course keep them when used for *Completions* display).
>
>
>         Stefan

On 2011-12-01 23:57 +0800, Stefan Monnier wrote:
[snipped 8 lines]
> I guess it could make sense to keep the face on the inserted text while
> you're still cycling, but it should be removed afterwards.  And since
> the transition between "cycling" and "not cycling" is not explicit,
> you'd then need/want to remove the face from something like
> a pre-command-hook.
>
>
>         Stefan

Hello Stefan,

I have been using completion with completion-cycle-threshold set to 4
and I have been annoyed often enough that I think it is worthwhile to
fix this bug because it often leaves my buffer weirdly fontified. For
example in elisp, `defma' can complete to `defmacro' unfontified.

[emacs-comp-bug.png (image/png, inline)]

[emacs-comp-correct.png (image/png, attachment)]

[Message part 4 (text/plain, inline)]

I propose the attached patch following your advice i.e. strip faces when
used for completion. Could you take a look at it? Thanks.

[mb.diff (text/x-diff, inline)]

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2012-01-05 09:46:05 +0000
+++ lisp/minibuffer.el	2012-01-16 09:05:45 +0000
@@ -1180,6 +1180,9 @@
 of the differing parts is, by contrast, slightly highlighted."
   :group 'completion)
 
+(defvar completion-hilit-commonality-p nil
+  "Internal variable. Bound to t in `minibuffer-completion-help'.")
+
 (defun completion-hilit-commonality (completions prefix-len base-size)
   (when completions
     (let ((com-str-len (- prefix-len (or base-size 0))))
@@ -1195,17 +1198,18 @@
                      (car (setq elem (cons (copy-sequence (car elem))
                                            (cdr elem))))
                    (setq elem (copy-sequence elem)))))
-            (put-text-property 0
-			       ;; If completion-boundaries returns incorrect
-			       ;; values, all-completions may return strings
-			       ;; that don't contain the prefix.
-			       (min com-str-len (length str))
-                               'font-lock-face 'completions-common-part
-                               str)
-            (if (> (length str) com-str-len)
-                (put-text-property com-str-len (1+ com-str-len)
-                                   'font-lock-face 'completions-first-difference
-                                   str)))
+            (when completion-hilit-commonality-p
+              (put-text-property 0
+                                 ;; If completion-boundaries returns incorrect
+                                 ;; values, all-completions may return strings
+                                 ;; that don't contain the prefix.
+                                 (min com-str-len (length str))
+                                 'font-lock-face 'completions-common-part
+                                 str)
+              (if (> (length str) com-str-len)
+                  (put-text-property com-str-len (1+ com-str-len)
+                                     'font-lock-face 'completions-first-difference
+                                     str))))
           elem)
         completions)
        base-size))))
@@ -1314,12 +1318,13 @@
          (end (field-end))
          (string (field-string))
          (md (completion--field-metadata start))
-         (completions (completion-all-completions
-                       string
-                       minibuffer-completion-table
-                       minibuffer-completion-predicate
-                       (- (point) (field-beginning))
-                       md)))
+         (completions (let ((completion-hilit-commonality-p t))
+                        (completion-all-completions
+                         string
+                         minibuffer-completion-table
+                         minibuffer-completion-predicate
+                         (- (point) (field-beginning))
+                         md))))
     (message nil)
     (if (or (null completions)
             (and (not (consp (cdr completions)))
@@ -2411,14 +2416,15 @@
          (setq str (copy-sequence str))
          (unless (string-match re str)
            (error "Internal error: %s does not match %s" re str))
-         (let ((pos (or (match-beginning 1) (match-end 0))))
-           (put-text-property 0 pos
-                              'font-lock-face 'completions-common-part
-                              str)
-           (if (> (length str) pos)
-               (put-text-property pos (1+ pos)
-				  'font-lock-face 'completions-first-difference
-				  str)))
+         (when completion-hilit-commonality-p
+           (let ((pos (or (match-beginning 1) (match-end 0))))
+             (put-text-property 0 pos
+                                'font-lock-face 'completions-common-part
+                                str)
+             (if (> (length str) pos)
+                 (put-text-property pos (1+ pos)
+                                    'font-lock-face 'completions-first-difference
+                                    str))))
 	 str)
        completions))))
 

[Message part 6 (text/plain, inline)]

Another simpler fix is to replace 'font-lock-face with 'face which would
then allow the major-mode to override the faces added by completion.

Leo

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo <sdl.web <at> gmail.com>
Cc: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Mon, 16 Jan 2012 10:52:07 -0500

> I propose the attached patch following your advice i.e. strip faces when
> used for completion. Could you take a look at it? Thanks.

Rather than prevent faces from being added, I installed a patch which
strips them before insertion.


        Stefan


--- lisp/minibuffer.el	2012-01-05 09:46:05 +0000
+++ lisp/minibuffer.el	2012-01-16 15:41:07 +0000
@@ -571,6 +571,10 @@
 (defun completion--replace (beg end newtext)
   "Replace the buffer text between BEG and END with NEWTEXT.
 Moves point to the end of the new text."
+  ;; The properties on `newtext' include things like
+  ;; completions-first-difference, which we don't want to include
+  ;; upon insertion.
+  (set-text-properties 0 (length newtext) nil newtext)
   ;; Maybe this should be in subr.el.
   ;; You'd think this is trivial to do, but details matter if you want
   ;; to keep markers "at the right place" and be robust in the face of

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

From: Leo <sdl.web <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 10062 <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Tue, 17 Jan 2012 12:29:28 +0800

On 2012-01-16 23:52 +0800, Stefan Monnier wrote:
> Rather than prevent faces from being added, I installed a patch which
> strips them before insertion.
>
>
>         Stefan
>
>
> --- lisp/minibuffer.el	2012-01-05 09:46:05 +0000
> +++ lisp/minibuffer.el	2012-01-16 15:41:07 +0000
> @@ -571,6 +571,10 @@
>  (defun completion--replace (beg end newtext)
>    "Replace the buffer text between BEG and END with NEWTEXT.
>  Moves point to the end of the new text."
> +  ;; The properties on `newtext' include things like
> +  ;; completions-first-difference, which we don't want to include
> +  ;; upon insertion.
> +  (set-text-properties 0 (length newtext) nil newtext)
>    ;; Maybe this should be in subr.el.
>    ;; You'd think this is trivial to do, but details matter if you want
>    ;; to keep markers "at the right place" and be robust in the face of

Thank you. This is a better fix. I think this bug can be closed.

Leo

Message #31 received at 10062-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Leo <sdl.web <at> gmail.com>
Cc: 10062-done <at> debbugs.gnu.org
Subject: Re: bug#10062: 24.0.91; completions-first-difference
Date: Tue, 17 Jan 2012 09:07:25 -0500

> Thank you. This is a better fix. I think this bug can be closed.

Thanks, done,


        Stefan "not convinced it's a better fix, but he prefers it anyway"

Previous Next

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