[-- 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 --]
> `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)
[-- 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)))))
> 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?
>> @@ -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.
> 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.
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.
> 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.