* bug#10062: 24.0.91; completions-first-difference @ 2011-11-16 9:45 Leo 2011-11-17 16:25 ` Leo 0 siblings, 1 reply; 9+ messages in thread From: Leo @ 2011-11-16 9:45 UTC (permalink / raw) To: 10062 [-- Attachment #1: Type: text/plain, Size: 193 bytes --] 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: [-- Attachment #2: emacs-completion-face.png --] [-- Type: image/png, Size: 4639 bytes --] [-- Attachment #3: Type: text/plain, Size: 195 bytes --] 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: [-- Attachment #4: emacs-slime-repl.png --] [-- Type: image/png, Size: 5470 bytes --] [-- Attachment #5: Type: text/plain, Size: 311 bytes --] 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'' ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2011-11-16 9:45 bug#10062: 24.0.91; completions-first-difference Leo @ 2011-11-17 16:25 ` Leo 2011-11-18 1:47 ` Stefan Monnier 2011-12-01 4:20 ` Leo 0 siblings, 2 replies; 9+ messages in thread From: Leo @ 2011-11-17 16:25 UTC (permalink / raw) To: 10062 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2011-11-17 16:25 ` Leo @ 2011-11-18 1:47 ` Stefan Monnier 2012-01-16 9:18 ` Leo 2011-12-01 4:20 ` Leo 1 sibling, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2011-11-18 1:47 UTC (permalink / raw) To: Leo; +Cc: 10062 >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2011-11-18 1:47 ` Stefan Monnier @ 2012-01-16 9:18 ` Leo 2012-01-16 15:52 ` Stefan Monnier 0 siblings, 1 reply; 9+ messages in thread From: Leo @ 2012-01-16 9:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: 10062 [-- Attachment #1: Type: text/plain, Size: 984 bytes --] 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. [-- Attachment #2: emacs-comp-bug.png --] [-- Type: image/png, Size: 7596 bytes --] [-- Attachment #3: emacs-comp-correct.png --] [-- Type: image/png, Size: 7782 bytes --] [-- Attachment #4: Type: text/plain, Size: 133 bytes --] I propose the attached patch following your advice i.e. strip faces when used for completion. Could you take a look at it? Thanks. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #5: mb.diff --] [-- Type: text/x-diff, Size: 4011 bytes --] === 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)))) [-- Attachment #6: Type: text/plain, Size: 148 bytes --] 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2012-01-16 9:18 ` Leo @ 2012-01-16 15:52 ` Stefan Monnier 2012-01-17 4:29 ` Leo 0 siblings, 1 reply; 9+ messages in thread From: Stefan Monnier @ 2012-01-16 15:52 UTC (permalink / raw) To: Leo; +Cc: 10062 > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2012-01-16 15:52 ` Stefan Monnier @ 2012-01-17 4:29 ` Leo 2012-01-17 14:07 ` Stefan Monnier 0 siblings, 1 reply; 9+ messages in thread From: Leo @ 2012-01-17 4:29 UTC (permalink / raw) To: Stefan Monnier; +Cc: 10062 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2012-01-17 4:29 ` Leo @ 2012-01-17 14:07 ` Stefan Monnier 0 siblings, 0 replies; 9+ messages in thread From: Stefan Monnier @ 2012-01-17 14:07 UTC (permalink / raw) To: Leo; +Cc: 10062-done > 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" ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2011-11-17 16:25 ` Leo 2011-11-18 1:47 ` Stefan Monnier @ 2011-12-01 4:20 ` Leo 2011-12-01 15:57 ` Stefan Monnier 1 sibling, 1 reply; 9+ messages in thread From: Leo @ 2011-12-01 4:20 UTC (permalink / raw) To: 10062 > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10062: 24.0.91; completions-first-difference 2011-12-01 4:20 ` Leo @ 2011-12-01 15:57 ` Stefan Monnier 0 siblings, 0 replies; 9+ messages in thread From: Stefan Monnier @ 2011-12-01 15:57 UTC (permalink / raw) To: Leo; +Cc: 10062 >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-01-17 14:07 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-11-16 9:45 bug#10062: 24.0.91; completions-first-difference Leo 2011-11-17 16:25 ` Leo 2011-11-18 1:47 ` Stefan Monnier 2012-01-16 9:18 ` Leo 2012-01-16 15:52 ` Stefan Monnier 2012-01-17 4:29 ` Leo 2012-01-17 14:07 ` Stefan Monnier 2011-12-01 4:20 ` Leo 2011-12-01 15:57 ` Stefan Monnier
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).
![[-- Attachment #2: emacs-comp-bug.png --] [-- Type: image/png, Size: 7596 bytes --]](../../m1hazwc8e9.fsf@gmail.com/2-emacs-comp-bug.png){kind=link}
![[-- Attachment #3: emacs-comp-correct.png --] [-- Type: image/png, Size: 7782 bytes --]](../../m1hazwc8e9.fsf@gmail.com/3-emacs-comp-correct.png){kind=link}