* bug#8897: `completion--insert-strings' clobbers user-added text properties
@ 2011-06-19 18:22 Štěpán Němec
2011-06-19 18:34 ` Glenn Morris
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Štěpán Němec @ 2011-06-19 18:22 UTC (permalink / raw
To: 8897
Tags: patch
It is possible to bind `completion-annotate-function' to add custom
annotations, which is great. Unfortunately, the `face' and `mouse-face'
text properties added by such a function are then unconditionally
overwritten by `completion--insert-strings'.
In my particular case I define annotations as buttons (which display
even more detail about a completion value upon activation), so a visual
indication of clickability is very important for me.
I wonder if something like the patch below, which fixes the problem for
me, could be applied?
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 284cbdc..11534e6 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -1003,9 +1003,12 @@ (defun completion--insert-strings (strings)
'mouse-face 'highlight)
(put-text-property (point) (progn (insert (car str)) (point))
'mouse-face 'highlight)
- (add-text-properties (point) (progn (insert (cadr str)) (point))
- '(mouse-face nil
- face completions-annotations)))
+ (let ((annotation (cadr str)))
+ (if (text-properties-at 1 annotation)
+ (insert annotation)
+ (add-text-properties (point) (progn (insert annotation) (point))
+ '(mouse-face nil
+ face completions-annotations)))))
(cond
((eq completions-format 'vertical)
;; Vertical format
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-19 18:22 bug#8897: `completion--insert-strings' clobbers user-added text properties Štěpán Němec
@ 2011-06-19 18:34 ` Glenn Morris
2011-06-20 3:08 ` Stefan Monnier
2020-09-14 12:25 ` Lars Ingebrigtsen
2 siblings, 0 replies; 8+ messages in thread
From: Glenn Morris @ 2011-06-19 18:34 UTC (permalink / raw
To: stepnem; +Cc: 8897
> I wonder if something like the patch below, which fixes the problem for
> me, could be applied?
We cannot apply any (more) patches from you because you don't want to
complete a copyright assignment. That's your prerogative of course, but
given that, it is best if you do not send patches when you report issues.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-19 18:22 bug#8897: `completion--insert-strings' clobbers user-added text properties Štěpán Němec
2011-06-19 18:34 ` Glenn Morris
@ 2011-06-20 3:08 ` Stefan Monnier
2011-06-20 8:07 ` Štěpán Němec
2020-09-14 12:25 ` Lars Ingebrigtsen
2 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2011-06-20 3:08 UTC (permalink / raw
To: Štěpán Němec; +Cc: 8897
> In my particular case I define annotations as buttons (which display
> even more detail about a completion value upon activation), so a visual
> indication of clickability is very important for me.
Currently, the completion-list-mode uses the `mouse-face' text-property
to determine what is a completion item and what i something else (blank
space, annotation, you name it). So as it stands, any mouse-face you'd
add to an annotation would confuse completion-list-mode into thinking it
is a completion item.
Maybe you could try to use the `category' text-property instead.
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-20 3:08 ` Stefan Monnier
@ 2011-06-20 8:07 ` Štěpán Němec
2011-06-20 14:01 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Štěpán Němec @ 2011-06-20 8:07 UTC (permalink / raw
To: Stefan Monnier; +Cc: 8897
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> In my particular case I define annotations as buttons (which display
>> even more detail about a completion value upon activation), so a visual
>> indication of clickability is very important for me.
>
> Currently, the completion-list-mode uses the `mouse-face' text-property
> to determine what is a completion item and what i something else (blank
> space, annotation, you name it). So as it stands, any mouse-face you'd
> add to an annotation would confuse completion-list-mode into thinking it
> is a completion item.
>
> Maybe you could try to use the `category' text-property instead.
Well I do use the `category' property, but the button functions seem to
set up the button face automatically, which is then clobbered by
`completion--insert-strings' (which is the problem). Here's how I set up
an annotation, `n' being the number of a pull request as a string (i.e.,
the completion value itself):
#+begin_src elisp
(let ((req (assoc-default n minibuffer-completion-table)))
(concat " "
(propertize (plist-get req :title)
'fontified t
'button '(t)
'category 'default-button
'help-echo "RET or mouse-2 for details"
'pr-data req
'action (lambda (b) (magithub-pull-request-details
(button-get b 'pr-data))))))
#+end_src
With my patch I haven't noticed any problems you mention -- clicking
or RET on the completion itself selects it, clicking or RET on the
annotation displays further details, the button face is preserved.
Štěpán
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-20 8:07 ` Štěpán Němec
@ 2011-06-20 14:01 ` Stefan Monnier
2011-06-20 14:22 ` Štěpán Němec
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2011-06-20 14:01 UTC (permalink / raw
To: Štěpán Němec; +Cc: 8897
>>> In my particular case I define annotations as buttons (which display
>>> even more detail about a completion value upon activation), so a visual
>>> indication of clickability is very important for me.
>> Currently, the completion-list-mode uses the `mouse-face' text-property
>> to determine what is a completion item and what i something else (blank
>> space, annotation, you name it). So as it stands, any mouse-face you'd
>> add to an annotation would confuse completion-list-mode into thinking it
>> is a completion item.
>> Maybe you could try to use the `category' text-property instead.
> Well I do use the `category' property, but the button functions seem to
> set up the button face automatically, which is then clobbered by
> `completion--insert-strings' (which is the problem). Here's how I set up
> an annotation, `n' being the number of a pull request as a string (i.e.,
> the completion value itself):
> #+begin_src elisp
> (let ((req (assoc-default n minibuffer-completion-table)))
> (concat " "
> (propertize (plist-get req :title)
> 'fontified t
> 'button '(t)
> 'category 'default-button
> 'help-echo "RET or mouse-2 for details"
> 'pr-data req
> 'action (lambda (b) (magithub-pull-request-details
> (button-get b 'pr-data))))))
> #+end_src
I must be missing something: where is a `face' or `mouse-face' property
added? The above code should not be affected by your patch, AFAICT.
And I don't understand your comment about "but the button functions seem
to set up the button face automatically" since I don't see where you
call a button function before insertion.
> With my patch I haven't noticed any problems you mention -- clicking
> or RET on the completion itself selects it, clicking or RET on the
> annotation displays further details, the button face is preserved.
I guess click&RET work OK because they're locally overridden, but if you
run M-x choose-completion while on the button or if you hit `left' or
`right' to skip from one completion to the next, you might see some
problem (although not with the code quoted above which should not
suffer from any of the problems discussed in this thread, AFAICT).
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-20 14:01 ` Stefan Monnier
@ 2011-06-20 14:22 ` Štěpán Němec
2011-06-20 16:36 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Štěpán Němec @ 2011-06-20 14:22 UTC (permalink / raw
To: Stefan Monnier; +Cc: 8897
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> #+begin_src elisp
>> (let ((req (assoc-default n minibuffer-completion-table)))
>> (concat " "
>> (propertize (plist-get req :title)
>> 'fontified t
>> 'button '(t)
>> 'category 'default-button
>> 'help-echo "RET or mouse-2 for details"
>> 'pr-data req
>> 'action (lambda (b) (magithub-pull-request-details
>> (button-get b 'pr-data))))))
>> #+end_src
>
> I must be missing something: where is a `face' or `mouse-face' property
> added? The above code should not be affected by your patch, AFAICT.
> And I don't understand your comment about "but the button functions seem
> to set up the button face automatically" since I don't see where you
> call a button function before insertion.
Sorry for my confusing description. What I meant is that there is
apparently some magic involved that makes a string manufactured with a
`propertize' form like the one above appear as if it had the button face
(underlined), even though it's not explicitely among its text properties
(maybe the `category' does it? I don't know).
In any case, without my patch the face is overridden to become the usual
annotation italic, so it _does_ suffer from the problem I describe.
>> With my patch I haven't noticed any problems you mention -- clicking
>> or RET on the completion itself selects it, clicking or RET on the
>> annotation displays further details, the button face is preserved.
>
> I guess click&RET work OK because they're locally overridden, but if you
> run M-x choose-completion while on the button or if you hit `left' or
> `right' to skip from one completion to the next, you might see some
> problem (although not with the code quoted above which should not
> suffer from any of the problems discussed in this thread, AFAICT).
Indeed. I don't use the cursor keys, but you're right in that they seem
to include the button-like annotations among their jump targets; as for
`choose-completion', that seems to be bound to RET in the *Completions*
buffer, but as I said and in line with your guess, pressing RET on an
annotation actually invokes the button action as desired, so this
problem doesn't seem to occur.
Štěpán
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-20 14:22 ` Štěpán Němec
@ 2011-06-20 16:36 ` Stefan Monnier
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2011-06-20 16:36 UTC (permalink / raw
To: Štěpán Němec; +Cc: 8897
>>> #+begin_src elisp
>>> (let ((req (assoc-default n minibuffer-completion-table)))
>>> (concat " "
>>> (propertize (plist-get req :title)
>>> 'fontified t
>>> 'button '(t)
>>> 'category 'default-button
>>> 'help-echo "RET or mouse-2 for details"
>>> 'pr-data req
>>> 'action (lambda (b) (magithub-pull-request-details
>>> (button-get b 'pr-data))))))
>>> #+end_src
>>
>> I must be missing something: where is a `face' or `mouse-face' property
>> added? The above code should not be affected by your patch, AFAICT.
>> And I don't understand your comment about "but the button functions seem
>> to set up the button face automatically" since I don't see where you
>> call a button function before insertion.
> Sorry for my confusing description. What I meant is that there is
> apparently some magic involved that makes a string manufactured with a
> `propertize' form like the one above appear as if it had the button face
> (underlined), even though it's not explicitely among its text properties
> (maybe the `category' does it? I don't know).
Yes, the `category' property is special and is "equivalent" to adding
all the properties on the symbol used for the category (i.e. all the
properties on the `default-button' symbol in your above example), which
most likely includes `face' and/or `mouse-face' properties.
> In any case, without my patch the face is overridden to become the usual
> annotation italic, so it _does_ suffer from the problem I describe.
Right, the `face' text-property takes precedence over the `face'
property provided by the `category' text property. Hmm...
> Indeed. I don't use the cursor keys, but you're right in that they seem
> to include the button-like annotations among their jump targets; as for
> `choose-completion', that seems to be bound to RET in the *Completions*
> buffer, but as I said and in line with your guess, pressing RET on an
> annotation actually invokes the button action as desired, so this
> problem doesn't seem to occur.
Yes, when point is on the button, RET (and mouse clicks) does not call
choose-completion but the button command instead. But if you have bound
choose-completion to some other keys that are not overridden by the
button, you'll see the problem (or if you do M-x choose-completion).
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#8897: `completion--insert-strings' clobbers user-added text properties
2011-06-19 18:22 bug#8897: `completion--insert-strings' clobbers user-added text properties Štěpán Němec
2011-06-19 18:34 ` Glenn Morris
2011-06-20 3:08 ` Stefan Monnier
@ 2020-09-14 12:25 ` Lars Ingebrigtsen
2 siblings, 0 replies; 8+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-14 12:25 UTC (permalink / raw
To: Štěpán Němec; +Cc: 8897
Štěpán Němec <stepnem@gmail.com> writes:
> It is possible to bind `completion-annotate-function' to add custom
> annotations, which is great. Unfortunately, the `face' and `mouse-face'
> text properties added by such a function are then unconditionally
> overwritten by `completion--insert-strings'.
>
> In my particular case I define annotations as buttons (which display
> even more detail about a completion value upon activation), so a visual
> indication of clickability is very important for me.
>
> I wonder if something like the patch below, which fixes the problem for
> me, could be applied?
In conjunction with bug#43218, completion no longer clobbers all text
properties (but it does clobber the face text property). So it still
requires some wrangling by the caller if the text property was the one
they wanted to have preserved, but the caller can stash that in a
different text property.
This requires that `minibuffer-allow-text-properties' is non-nil.
So I think that basically takes care of the problem described here, and
I'm closing this bug report.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-14 12:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-19 18:22 bug#8897: `completion--insert-strings' clobbers user-added text properties Štěpán Němec
2011-06-19 18:34 ` Glenn Morris
2011-06-20 3:08 ` Stefan Monnier
2011-06-20 8:07 ` Štěpán Němec
2011-06-20 14:01 ` Stefan Monnier
2011-06-20 14:22 ` Štěpán Němec
2011-06-20 16:36 ` Stefan Monnier
2020-09-14 12:25 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.