* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
@ 2021-05-26 12:50 João Guerra
2021-05-31 20:24 ` Juri Linkov
2021-06-05 21:43 ` Juri Linkov
0 siblings, 2 replies; 8+ messages in thread
From: João Guerra @ 2021-05-26 12:50 UTC (permalink / raw)
To: 48669
[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]
`minibuffer-message' and `set-minibuffer-message' place the overlay
containing the message slightly differently, resulting in a different
behaviour when there are other overlays present at the end of the
minibuffer.
The overlay created by `minibuffer-message' always gets added to the
end of the minibuffer.
The overlay created by `set-minibuffer-message' also gets added to the
end of the minibuffer, but its properties make it so that it is shown
before other overlays at the position.
This is particularly important when the user uses packages that add
overlays to the minibuffer, such as vertical completion packages like
selectrum, vertico, and others.
The way `set-minibuffer-message' configures the overlays makes it so
that the overlay is displayed before the other overlays, which I
believe is more compatible with external packages (see attached image
for a visual example).
Would it make sense to change this behaviour (i.e. overlay
configuration) so that it is consistent in both functions?
[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 21562 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-05-26 12:50 bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message João Guerra
@ 2021-05-31 20:24 ` Juri Linkov
2021-06-05 21:43 ` Juri Linkov
1 sibling, 0 replies; 8+ messages in thread
From: Juri Linkov @ 2021-05-31 20:24 UTC (permalink / raw)
To: João Guerra; +Cc: 48669
> `minibuffer-message' and `set-minibuffer-message' place the overlay
> containing the message slightly differently, resulting in a different
> behaviour when there are other overlays present at the end of the
> minibuffer.
>
> The overlay created by `minibuffer-message' always gets added to the
> end of the minibuffer.
> The overlay created by `set-minibuffer-message' also gets added to the
> end of the minibuffer, but its properties make it so that it is shown
> before other overlays at the position.
> This is particularly important when the user uses packages that add
> overlays to the minibuffer, such as vertical completion packages like
> selectrum, vertico, and others.
> The way `set-minibuffer-message' configures the overlays makes it so
> that the overlay is displayed before the other overlays, which I
> believe is more compatible with external packages (see attached image
> for a visual example).
To support completion packages such as icomplete, the overlay in
`set-minibuffer-message' was fixed, but not `minibuffer-message'.
> Would it make sense to change this behaviour (i.e. overlay
> configuration) so that it is consistent in both functions?
Right, `minibuffer-message' should be consistent with `set-minibuffer-message'
by copying this part:
;; Make sure the overlay with the message is displayed before
;; any other overlays in that position, in case they have
;; resize-mini-windows set to nil and the other overlay strings
;; are too long for the mini-window width. This makes sure the
;; temporary message will always be visible.
(overlay-put minibuffer-message-overlay 'priority 1100)
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-05-26 12:50 bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message João Guerra
2021-05-31 20:24 ` Juri Linkov
@ 2021-06-05 21:43 ` Juri Linkov
2021-06-06 5:58 ` Eli Zaretskii
1 sibling, 1 reply; 8+ messages in thread
From: Juri Linkov @ 2021-06-05 21:43 UTC (permalink / raw)
To: João Guerra; +Cc: 48669
[-- Attachment #1: Type: text/plain, Size: 309 bytes --]
> Would it make sense to change this behaviour (i.e. overlay
> configuration) so that it is consistent in both functions?
Thanks for the very useful suggestion. This could be fixed
with the following patch where I tried to sync everything
overlay-related from set-minibuffer-message to minibuffer-message:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuffer-message-overlay.patch --]
[-- Type: text/x-diff, Size: 1728 bytes --]
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index ffb74235e8..cb12226c07 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -741,7 +741,8 @@ minibuffer-message
;; Don't overwrite the face properties the caller has set
(text-properties-at 0 message))
(setq message (apply #'propertize message minibuffer-message-properties)))
- (let ((ol (make-overlay (point-max) (point-max) nil t t))
+ (let* ((ovpos (minibuffer--message-overlay-pos))
+ (ol (make-overlay ovpos ovpos nil t t))
;; A quit during sit-for normally only interrupts the sit-for,
;; but since minibuffer-message is used at the end of a command,
;; at a time when the command has virtually finished already, a C-g
@@ -755,8 +756,14 @@ minibuffer-message
;; The current C cursor code doesn't know to use the overlay's
;; marker's stickiness to figure out whether to place the cursor
;; before or after the string, so let's spoon-feed it the pos.
- (put-text-property 0 1 'cursor t message))
+ (put-text-property 0 1 'cursor 1 message))
(overlay-put ol 'after-string message)
+ ;; Make sure the overlay with the message is displayed before
+ ;; any other overlays in that position, in case they have
+ ;; resize-mini-windows set to nil and the other overlay strings
+ ;; are too long for the mini-window width. This makes sure the
+ ;; temporary message will always be visible.
+ (overlay-put ol 'priority 1100)
(sit-for (or minibuffer-message-timeout 1000000)))
(delete-overlay ol)))))
^ permalink raw reply related [flat|nested] 8+ messages in thread* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-06-05 21:43 ` Juri Linkov
@ 2021-06-06 5:58 ` Eli Zaretskii
2021-06-06 20:54 ` Juri Linkov
0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2021-06-06 5:58 UTC (permalink / raw)
To: Juri Linkov; +Cc: joca.bt, 48669
> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 06 Jun 2021 00:43:54 +0300
> Cc: 48669@debbugs.gnu.org
>
>
> [1:text/plain Hide]
>
> > Would it make sense to change this behaviour (i.e. overlay
> > configuration) so that it is consistent in both functions?
>
> Thanks for the very useful suggestion. This could be fixed
> with the following patch where I tried to sync everything
> overlay-related from set-minibuffer-message to minibuffer-message:
>
>
> [2:text/x-diff Hide]
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index ffb74235e8..cb12226c07 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -741,7 +741,8 @@ minibuffer-message
> ;; Don't overwrite the face properties the caller has set
> (text-properties-at 0 message))
> (setq message (apply #'propertize message minibuffer-message-properties)))
> - (let ((ol (make-overlay (point-max) (point-max) nil t t))
> + (let* ((ovpos (minibuffer--message-overlay-pos))
> + (ol (make-overlay ovpos ovpos nil t t))
Doesn't this rely too much on the internal details of
minibuffer--message-overlay-pos? At least, without any comments, this
call looks like a riddle that isn't easy to unlock.
> - (put-text-property 0 1 'cursor t message))
> + (put-text-property 0 1 'cursor 1 message))
Why this change?
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-06-06 5:58 ` Eli Zaretskii
@ 2021-06-06 20:54 ` Juri Linkov
2021-06-07 12:21 ` Eli Zaretskii
0 siblings, 1 reply; 8+ messages in thread
From: Juri Linkov @ 2021-06-06 20:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: joca.bt, 48669
>> @@ -741,7 +741,8 @@ minibuffer-message
>> ;; Don't overwrite the face properties the caller has set
>> (text-properties-at 0 message))
>> (setq message (apply #'propertize message minibuffer-message-properties)))
>> - (let ((ol (make-overlay (point-max) (point-max) nil t t))
>> + (let* ((ovpos (minibuffer--message-overlay-pos))
>> + (ol (make-overlay ovpos ovpos nil t t))
>
> Doesn't this rely too much on the internal details of
> minibuffer--message-overlay-pos? At least, without any comments, this
> call looks like a riddle that isn't easy to unlock.
If minibuffer--message-overlay-pos serves its purpose for
set-minibuffer-message, it seems suitable for
minibuffer-message as well.
>> - (put-text-property 0 1 'cursor t message))
>> + (put-text-property 0 1 'cursor 1 message))
>
> Why this change?
Only to make minibuffer-message the identical copy of
set-minibuffer-message.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-06-06 20:54 ` Juri Linkov
@ 2021-06-07 12:21 ` Eli Zaretskii
2021-06-08 16:52 ` Juri Linkov
0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2021-06-07 12:21 UTC (permalink / raw)
To: Juri Linkov; +Cc: joca.bt, 48669
> From: Juri Linkov <juri@linkov.net>
> Cc: joca.bt@gmail.com, 48669@debbugs.gnu.org
> Date: Sun, 06 Jun 2021 23:54:58 +0300
>
> >> - (let ((ol (make-overlay (point-max) (point-max) nil t t))
> >> + (let* ((ovpos (minibuffer--message-overlay-pos))
> >> + (ol (make-overlay ovpos ovpos nil t t))
> >
> > Doesn't this rely too much on the internal details of
> > minibuffer--message-overlay-pos? At least, without any comments, this
> > call looks like a riddle that isn't easy to unlock.
>
> If minibuffer--message-overlay-pos serves its purpose for
> set-minibuffer-message, it seems suitable for
> minibuffer-message as well.
So you object to adding a comment there which would explain what that
call does?
> >> - (put-text-property 0 1 'cursor t message))
> >> + (put-text-property 0 1 'cursor 1 message))
> >
> > Why this change?
>
> Only to make minibuffer-message the identical copy of
> set-minibuffer-message.
I'd say make it the other way around. Handling the 'cursor' property
with a numeric value is more expensive and tricky than if the value is
t, and in this case the effect is exactly the same.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-06-07 12:21 ` Eli Zaretskii
@ 2021-06-08 16:52 ` Juri Linkov
2021-06-08 18:12 ` Eli Zaretskii
0 siblings, 1 reply; 8+ messages in thread
From: Juri Linkov @ 2021-06-08 16:52 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: joca.bt, 48669
tags 48669 fixed
close 48669 28.0.50
thanks
>> >> - (let ((ol (make-overlay (point-max) (point-max) nil t t))
>> >> + (let* ((ovpos (minibuffer--message-overlay-pos))
>> >> + (ol (make-overlay ovpos ovpos nil t t))
>> >
>> > Doesn't this rely too much on the internal details of
>> > minibuffer--message-overlay-pos? At least, without any comments, this
>> > call looks like a riddle that isn't easy to unlock.
>>
>> If minibuffer--message-overlay-pos serves its purpose for
>> set-minibuffer-message, it seems suitable for
>> minibuffer-message as well.
>
> So you object to adding a comment there which would explain what that
> call does?
Now I added a comment, and pushed to master.
>> >> - (put-text-property 0 1 'cursor t message))
>> >> + (put-text-property 0 1 'cursor 1 message))
>> >
>> > Why this change?
>>
>> Only to make minibuffer-message the identical copy of
>> set-minibuffer-message.
>
> I'd say make it the other way around. Handling the 'cursor' property
> with a numeric value is more expensive and tricky than if the value is
> t, and in this case the effect is exactly the same.
So I changed the 'cursor' property to t.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message
2021-06-08 16:52 ` Juri Linkov
@ 2021-06-08 18:12 ` Eli Zaretskii
0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2021-06-08 18:12 UTC (permalink / raw)
To: Juri Linkov; +Cc: joca.bt, 48669
> From: Juri Linkov <juri@linkov.net>
> Cc: joca.bt@gmail.com, 48669@debbugs.gnu.org
> Date: Tue, 08 Jun 2021 19:52:07 +0300
>
> >> If minibuffer--message-overlay-pos serves its purpose for
> >> set-minibuffer-message, it seems suitable for
> >> minibuffer-message as well.
> >
> > So you object to adding a comment there which would explain what that
> > call does?
>
> Now I added a comment, and pushed to master.
>
> >> >> - (put-text-property 0 1 'cursor t message))
> >> >> + (put-text-property 0 1 'cursor 1 message))
> >> >
> >> > Why this change?
> >>
> >> Only to make minibuffer-message the identical copy of
> >> set-minibuffer-message.
> >
> > I'd say make it the other way around. Handling the 'cursor' property
> > with a numeric value is more expensive and tricky than if the value is
> > t, and in this case the effect is exactly the same.
>
> So I changed the 'cursor' property to t.
Thank you.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-08 18:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-26 12:50 bug#48669: Inconsistent overlay placement between minibuffer-message and set-minibuffer-message João Guerra
2021-05-31 20:24 ` Juri Linkov
2021-06-05 21:43 ` Juri Linkov
2021-06-06 5:58 ` Eli Zaretskii
2021-06-06 20:54 ` Juri Linkov
2021-06-07 12:21 ` Eli Zaretskii
2021-06-08 16:52 ` Juri Linkov
2021-06-08 18:12 ` Eli Zaretskii
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).