bug#38457: 27.0.50; dabbrev-expand regression due to message change

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stephen Berman]

0. emacs -Q
1. M-! te M-/

=> It takes about 4 seconds on my machine for the expansion "text" to
appear, during which the minibuffer displays "[Scanning for
dabbrevs...100%]" and then "[Scanning for dabbrevs...done]".  On builds
without this regression, the expansion is virtually instantaneous and no
message is seen in the minibuffer.

Git bisect pinpoints the following change:

aa89c84e00d8dc85100e6fedab7631c415e6364d is the first bad commit
commit aa89c84e00d8dc85100e6fedab7631c415e6364d
Author: Juri Linkov <juri@linkov.net>
Date:   Wed Nov 27 01:43:49 2019 +0200

    message uses minibuffer-message in the active minibuffer (bug#17272 bug#19064)

    * doc/lispref/display.texi (Displaying Messages): Explain the
    behavior of using minibuffer-message if the minibuffer is active.

    * src/editfns.c (Fmessage_in_echo_area): New function with body
    copied from Fmessage.
    (Fmessage): Call minibuffer-message in the active minibuffer,
    otherwise call Fmessage_in_echo_area.
    (message-in-echo-area): New variable.

    * lisp/isearch.el (isearch--momentary-message, isearch-message):
    * lisp/minibuffer.el (minibuffer-message, minibuffer-completion-help):
    Use 'message-in-echo-area' instead of 'message' where necessary.

    * lisp/autorevert.el (auto-revert-handler):
    * lisp/man.el (Man-bgproc-sentinel):
    * lisp/subr.el (do-after-load-evaluation):
    Revert recent changes that replaced 'message' with 'minibuffer-message'.
    This is not needed anymore since 'message' uses 'minibuffer-message'
    in the active minibuffer.


In GNU Emacs 27.0.50 (build 27, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
 of 2019-12-01 built on strobe-lfs84
Repository revision: 9f2145f42daab13aed5cf89fdb6a7c5579819ec0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12003000
System Description: Linux From Scratch

Configured using:
 'configure --with-cairo 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS JSON PDUMPER LCMS2
GMP

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Mon, 02 Dec 2019 12:06:55 +0100
> 
> 0. emacs -Q
> 1. M-! te M-/
> 
> => It takes about 4 seconds on my machine for the expansion "text" to
> appear, during which the minibuffer displays "[Scanning for
> dabbrevs...100%]" and then "[Scanning for dabbrevs...done]".  On builds
> without this regression, the expansion is virtually instantaneous and no
> message is seen in the minibuffer.
> 
> Git bisect pinpoints the following change:
> 
> aa89c84e00d8dc85100e6fedab7631c415e6364d is the first bad commit
> commit aa89c84e00d8dc85100e6fedab7631c415e6364d
> Author: Juri Linkov <juri@linkov.net>
> Date:   Wed Nov 27 01:43:49 2019 +0200
> 
>     message uses minibuffer-message in the active minibuffer (bug#17272 bug#19064)
> 
>     * doc/lispref/display.texi (Displaying Messages): Explain the
>     behavior of using minibuffer-message if the minibuffer is active.
> 
>     * src/editfns.c (Fmessage_in_echo_area): New function with body
>     copied from Fmessage.
>     (Fmessage): Call minibuffer-message in the active minibuffer,
>     otherwise call Fmessage_in_echo_area.
>     (message-in-echo-area): New variable.
> 
>     * lisp/isearch.el (isearch--momentary-message, isearch-message):
>     * lisp/minibuffer.el (minibuffer-message, minibuffer-completion-help):
>     Use 'message-in-echo-area' instead of 'message' where necessary.
> 
>     * lisp/autorevert.el (auto-revert-handler):
>     * lisp/man.el (Man-bgproc-sentinel):
>     * lisp/subr.el (do-after-load-evaluation):
>     Revert recent changes that replaced 'message' with 'minibuffer-message'.
>     This is not needed anymore since 'message' uses 'minibuffer-message'
>     in the active minibuffer.

I guess the new message-in-echo-area confuses dabbrev.el because it
switches buffers, and dabbrev.el has special logic for that, which
triggers the message, and the subsequent wait.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

> 0. emacs -Q
> 1. M-! te M-/
>
> => It takes about 4 seconds on my machine for the expansion "text" to
> appear, during which the minibuffer displays "[Scanning for
> dabbrevs...100%]" and then "[Scanning for dabbrevs...done]".  On builds
> without this regression, the expansion is virtually instantaneous and no
> message is seen in the minibuffer.

This is because of the current limitation of minibuffer-message.
It uses sit-for to wait for 2 seconds per every message.
This should be fixed by using the timer, so there will be no delays
anymore:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuffer-message-timer.patch --]
[-- Type: text/x-diff, Size: 2894 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index a7bdde478f..3febdeb174 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -702,6 +702,9 @@ minibuffer
 (defvar minibuffer-message-properties nil
   "Text properties added to the text shown by `minibuffer-message'.")
 
+(defvar minibuffer-message-timer nil)
+(defvar minibuffer-message-overlay nil)
+
 (defun minibuffer-message (message &rest args)
   "Temporarily display MESSAGE at the end of the minibuffer.
 The text is displayed for `minibuffer-message-timeout' seconds,
@@ -732,24 +735,23 @@ 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))
-          ;; 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
-          ;; should really cause an abort-recursive-edit instead (i.e. as if
-          ;; the C-g had been typed at top-level).  Binding inhibit-quit here
-          ;; is an attempt to get that behavior.
-          (inhibit-quit t))
-      (unwind-protect
-          (progn
-            (unless (zerop (length 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))
-            (overlay-put ol 'after-string message)
-            (sit-for (or minibuffer-message-timeout 1000000)))
-        (delete-overlay ol)))))
+
+    (when (timerp minibuffer-message-timer)
+      (cancel-timer minibuffer-message-timer))
+    (when (overlayp minibuffer-message-overlay)
+      (delete-overlay minibuffer-message-overlay))
+    (setq minibuffer-message-overlay
+          (make-overlay (point-max) (point-max) nil t t))
+    (setq minibuffer-message-timer
+          (run-with-timer (or minibuffer-message-timeout 1) nil
+                          (lambda () (when (overlayp minibuffer-message-overlay)
+                                       (delete-overlay minibuffer-message-overlay)))))
+    (unless (zerop (length 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))
+    (overlay-put minibuffer-message-overlay 'after-string message)))
 
 (defun minibuffer-completion-contents ()
   "Return the user input in a minibuffer before point as a string.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stephen Berman]

On Tue, 03 Dec 2019 01:00:40 +0200 Juri Linkov <juri@linkov.net> wrote:

>> 0. emacs -Q
>> 1. M-! te M-/
>>
>> => It takes about 4 seconds on my machine for the expansion "text" to
>> appear, during which the minibuffer displays "[Scanning for
>> dabbrevs...100%]" and then "[Scanning for dabbrevs...done]".  On builds
>> without this regression, the expansion is virtually instantaneous and no
>> message is seen in the minibuffer.
>
> This is because of the current limitation of minibuffer-message.
> It uses sit-for to wait for 2 seconds per every message.
> This should be fixed by using the timer, so there will be no delays
> anymore:

I confirm that this patch eliminates the dabbrev-expand delay.
Simultaneously with the expansion, the message "[Scanning for
dabbrevs...done]" is displayed, which is uninformative and hence a bit
annoying, but otherwise seems harmless.  Thanks for the fix.

Steve Berman

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Tue, 03 Dec 2019 01:00:40 +0200
> Cc: 38457@debbugs.gnu.org
> 
> > 0. emacs -Q
> > 1. M-! te M-/
> >
> > => It takes about 4 seconds on my machine for the expansion "text" to
> > appear, during which the minibuffer displays "[Scanning for
> > dabbrevs...100%]" and then "[Scanning for dabbrevs...done]".  On builds
> > without this regression, the expansion is virtually instantaneous and no
> > message is seen in the minibuffer.
> 
> This is because of the current limitation of minibuffer-message.
> It uses sit-for to wait for 2 seconds per every message.
> This should be fixed by using the timer, so there will be no delays
> anymore:

Please don't make this change, or any other changes in this stuff that
significantly modify the internals of these basic APIs.  I'm that
close to ask to revert the entire message-in-echo-area change, because
it's already too invasive and runs a high risk of disrupting too many
commands.  We don't want the pretest of Emacs 27.1 to last forever for
these reasons.

So please try to find a simpler fix for this problem, one that doesn't
involve a timer.  If it isn't possible, let's indeed consider removing
the message-in-echo-area changes for now.

Sorry for that, but we should keep in mind that Emacs 27.1 pretest is
coming soon, and any deep changes in low-level infrastructure used all
over the place should wait for the next release.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> So please try to find a simpler fix for this problem, one that doesn't
> involve a timer.

This is the simplest and the safest solution.  I wanted to implement it
earlier but had no time before.  I see no problems with it.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: stephen.berman@gmx.net,  38457@debbugs.gnu.org
> Date: Wed, 04 Dec 2019 01:44:57 +0200
> 
> > So please try to find a simpler fix for this problem, one that doesn't
> > involve a timer.
> 
> This is the simplest and the safest solution.

The simplest solution would be to prevent dabbrev-expand from
displaying the message in the first place; there are conditions for
that which evidently the new implementation somehow triggers.

> I wanted to implement it earlier but had no time before.  I see no
> problems with it.

Let's not rock the boat too much so close to the release cycle because
of a very minor problem which caused these changes.  You may not see
any problems with it now, but I'm certain we will bump into numerous
problems very soon if we make such significant changes in this
infrastructure, which is used everywhere in Emacs.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> I wanted to implement it earlier but had no time before.  I see no
>> problems with it.
>
> Let's not rock the boat too much so close to the release cycle because
> of a very minor problem which caused these changes.  You may not see
> any problems with it now, but I'm certain we will bump into numerous
> problems very soon if we make such significant changes in this
> infrastructure, which is used everywhere in Emacs.

There may be a misunderstanding here.  I was talking about the
changes for development in master, not for the release branch.
If you think it would be too risky to release with these changes,
then fine, these changes could be removed in the release branch.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: stephen.berman@gmx.net,  38457@debbugs.gnu.org
> Date: Thu, 05 Dec 2019 01:16:34 +0200
> 
> > Let's not rock the boat too much so close to the release cycle because
> > of a very minor problem which caused these changes.  You may not see
> > any problems with it now, but I'm certain we will bump into numerous
> > problems very soon if we make such significant changes in this
> > infrastructure, which is used everywhere in Emacs.
> 
> There may be a misunderstanding here.  I was talking about the
> changes for development in master, not for the release branch.
> If you think it would be too risky to release with these changes,
> then fine, these changes could be removed in the release branch.

The master branch will become the release branch very soon.  If you
want to wait with these changes after the emacs-27 branch is cut, it's
fine by me, but we need to solve the issue with dabbrev-expand for the
release branch as well.

That is why I suggest to find a simpler, safer solution for that issue
now, and if you insist on using a timer (which I personally consider
not a good idea), then do this after the release branch is cut.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > Let's not rock the boat too much so close to the release cycle because
>> > of a very minor problem which caused these changes.  You may not see
>> > any problems with it now, but I'm certain we will bump into numerous
>> > problems very soon if we make such significant changes in this
>> > infrastructure, which is used everywhere in Emacs.
>>
>> There may be a misunderstanding here.  I was talking about the
>> changes for development in master, not for the release branch.
>> If you think it would be too risky to release with these changes,
>> then fine, these changes could be removed in the release branch.
>
> The master branch will become the release branch very soon.  If you
> want to wait with these changes after the emacs-27 branch is cut, it's
> fine by me, but we need to solve the issue with dabbrev-expand for the
> release branch as well.
>
> That is why I suggest to find a simpler, safer solution for that issue
> now, and if you insist on using a timer (which I personally consider
> not a good idea), then do this after the release branch is cut.

Using a timer is the right thing to do, there is no simpler solution,
it solves all problems.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: stephen.berman@gmx.net,  38457@debbugs.gnu.org
> Date: Fri, 06 Dec 2019 02:10:20 +0200
> 
> > That is why I suggest to find a simpler, safer solution for that issue
> > now, and if you insist on using a timer (which I personally consider
> > not a good idea), then do this after the release branch is cut.
> 
> Using a timer is the right thing to do, there is no simpler solution,
> it solves all problems.

Sorry, I object to installing a timer-based solution at this time.  It
is too radical and is likely to cause numerous unintended consequences
all over the place.

I just suggested another possible solution: bind
'minibuffer-message-timeout' to zero when calling 'minibuffer-message'
from 'message'.  Would that work, or will it have some unwanted side
effects?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > That is why I suggest to find a simpler, safer solution for that issue
>> > now, and if you insist on using a timer (which I personally consider
>> > not a good idea), then do this after the release branch is cut.
>> 
>> Using a timer is the right thing to do, there is no simpler solution,
>> it solves all problems.
>
> Sorry, I object to installing a timer-based solution at this time.  It
> is too radical and is likely to cause numerous unintended consequences
> all over the place.
>
> I just suggested another possible solution: bind
> 'minibuffer-message-timeout' to zero when calling 'minibuffer-message'
> from 'message'.  Would that work, or will it have some unwanted side
> effects?

Kévin posted a patch that binds 'minibuffer-message-timeout' to zero, but
it makes each message disappear before the user has a chance to read it.

With a timer-based approach each message disappears too, except
the last message that the user has a chance to read.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: stephen.berman@gmx.net,  38457@debbugs.gnu.org
> Date: Sun, 08 Dec 2019 01:05:03 +0200
> 
> > I just suggested another possible solution: bind
> > 'minibuffer-message-timeout' to zero when calling 'minibuffer-message'
> > from 'message'.  Would that work, or will it have some unwanted side
> > effects?
> 
> Kévin posted a patch that binds 'minibuffer-message-timeout' to zero, but
> it makes each message disappear before the user has a chance to read it.

But that's how 'message' behaved as well, didn't it?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

On December 8, 2019 5:28:31 AM GMT+02:00, Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Juri Linkov <juri@linkov.net>
> > Cc: stephen.berman@gmx.net,  38457@debbugs.gnu.org
> > Date: Sun, 08 Dec 2019 01:05:03 +0200
> > 
> > > I just suggested another possible solution: bind
> > > 'minibuffer-message-timeout' to zero when calling
> 'minibuffer-message'
> > > from 'message'.  Would that work, or will it have some unwanted
> side
> > > effects?
> > 
> > Kévin posted a patch that binds 'minibuffer-message-timeout' to
> zero, but
> > it makes each message disappear before the user has a chance to read
> it.
> 
> But that's how 'message' behaved as well, didn't it?

In case the answer is NO, here's an alternative proposal.

The original problem which the change in 'message' tried to solve, the one reported in bugs #17272 and #19064, was about calling 'message' when y-or-n-p is prompting the user.  So how about if we modify the conditions under which 'message' calls 'minibuffer-message' such that this happens only when y-or-n-p is in progress?  E.g., y-or-n-p could bind some variable that 'message' would check.

Maybe yes-or-no-p should do the same.

This will allow us to fix the original bugs without such wide implications as we have now.

And while at that, I don't think we need the new function message-on-echo-area; the additional logic could be added to 'message's original code.

WDYT?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > > I just suggested another possible solution: bind
>> > > 'minibuffer-message-timeout' to zero when calling
>> 'minibuffer-message'
>> > > from 'message'.  Would that work, or will it have some unwanted
>> side
>> > > effects?
>> >
>> > Kévin posted a patch that binds 'minibuffer-message-timeout' to
>> zero, but
>> > it makes each message disappear before the user has a chance to read
>> it.
>>
>> But that's how 'message' behaved as well, didn't it?
>
> In case the answer is NO, here's an alternative proposal.

Yes, the answer is NO.

> The original problem which the change in 'message' tried to solve, the one
> reported in bugs #17272 and #19064, was about calling 'message' when
> y-or-n-p is prompting the user.  So how about if we modify the conditions
> under which 'message' calls 'minibuffer-message' such that this happens
> only when y-or-n-p is in progress?  E.g., y-or-n-p could bind some variable
> that 'message' would check.

No, it was not only about y-or-n-p, but about any command that uses the minibuffer.

> Maybe yes-or-no-p should do the same.
>
> This will allow us to fix the original bugs without such wide implications as we have now.
>
> And while at that, I don't think we need the new function message-on-echo-area; the additional logic could be added to 'message's original code.
>
> WDYT?

Inventing hacks and workarounds is too risky and error-prone
than installing the proper and safest solution.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: bug-gnu-emacs@gnu.org,  38457@debbugs.gnu.org, stephen.berman@gmx.net
> Date: Sun, 08 Dec 2019 23:50:28 +0200
> 
> > The original problem which the change in 'message' tried to solve, the one
> > reported in bugs #17272 and #19064, was about calling 'message' when
> > y-or-n-p is prompting the user.  So how about if we modify the conditions
> > under which 'message' calls 'minibuffer-message' such that this happens
> > only when y-or-n-p is in progress?  E.g., y-or-n-p could bind some variable
> > that 'message' would check.
> 
> No, it was not only about y-or-n-p, but about any command that uses the minibuffer.

As it turns out, not any command, because "M-x" and "C-x C-f" also use
the minibuffer.

And the original bug reports definitely were about y-or-n-p.

> > Maybe yes-or-no-p should do the same.
> >
> > This will allow us to fix the original bugs without such wide implications as we have now.
> >
> > And while at that, I don't think we need the new function message-on-echo-area; the additional logic could be added to 'message's original code.
> >
> > WDYT?
> 
> Inventing hacks and workarounds is too risky and error-prone
> than installing the proper and safest solution.

I don't see how this is a "hack" when it uses the same technique as
your changes in 'message': checking a variable that is bound by other
functions.  The advantage of my proposal is that it makes the new
functionality opt-in, so that any commands which need this could have
it by simply binding a variable, and would otherwise maintain its old
behavior, which was there for eons.

This issue is basically the only one that's left before we can cut the
release branch.  Please help me resolve this issue so that we could
start the pretest of Emacs 27.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> As it turns out, not any command, because "M-x" and "C-x C-f" also use
> the minibuffer.
>
> And the original bug reports definitely were about y-or-n-p.

That's true, but I think the way it works now is a huge improvement.
Just about whatever I'm doing now, asynchronous messaging does not
overwrite prompts/M-x actions, and that's really nice.

The bug reports may have been about y-or-n-p, but it's a general problem
that's been solved in a general way now.  Reverting this would be a step
in the wrong direction.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Juri Linkov <juri@linkov.net>,  38457@debbugs.gnu.org,
>   stephen.berman@gmx.net
> Date: Mon, 09 Dec 2019 08:43:11 +0100
> 
> > As it turns out, not any command, because "M-x" and "C-x C-f" also use
> > the minibuffer.
> >
> > And the original bug reports definitely were about y-or-n-p.
> 
> That's true, but I think the way it works now is a huge improvement.
> Just about whatever I'm doing now, asynchronous messaging does not
> overwrite prompts/M-x actions, and that's really nice.
> 
> The bug reports may have been about y-or-n-p, but it's a general problem
> that's been solved in a general way now.  Reverting this would be a step
> in the wrong direction.

I didn't suggest to revert it, I suggested to modify it so that we
could selectively enable the feature where it doesn't have adverse
side effects.  By contrast, what we have now summarily enables the
feature for every use of the minibuffer, and we will then have to
somehow disable or augment it in every case where it does have adverse
side effects.  We already have 2 bug reports about it for 2 separate
packages, and we cannot leave them unsolved.  I'm sure this is just
the tip of a very large iceberg.

How do you propose we move forward?  You think we should install the
timer proposed by Juri instead?  That would make this change even more
invasive than it already is, and will again affect every use of the
minibuffer, and then some.  And I have no doubt that the delay in
removing the message is not the only side effect of the change in how
'message' now works, it's just the first one we noticed.  This is a
sure way to delay Emacs 27 even further.

We are developing Emacs 27 for more than 1.5 years.  We need to start
its release cycle very soon, and we should start from a stable code
base.  I have been asking people since July to avoid putting
destabilizing changes on master.  The change in 'message' destabilized
what we had before, and the proposed timer will destabilize it even
more.  How long do we want the Emacs 27.1 pretest to last? a year?
more?

We could also revert the change now and reapply it after the emacs-27
branch is cut.  That would mean the original use case with y-or-n-p
will not be fixed until Emacs 28.  Personally, I think it would be a
pity to leave that issue unsolved in Emacs 27, but if I cannot get us
to agree to any other solution, maybe that's what we should do.

More generally, I need everyone's help in making Emacs 27.1 happen
soon enough.  I cannot do it if people discount my gut feelings about
Emacs stability when those gut feelings are strong enough for me to
object to changes.  It's unfair to expect me to agree to something
when every bit of my experience screams that it's wrong.  If I cannot
get people to bear with me even on rare occasions when I have those
gut feelings, I guess I'm unfit for the job, and someone else should
take over.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

Here's one more problem with the new behavior of 'message':

  emacs -Q

  Evaluate:

  (defun my-mesage ()
    (interactive)
    (message "my most important message"))
  (global-set-key [f5] 'my-mesage)
  (setq debug-on-message "important")

  Press F5, observe the debugger pop up, as expected.
  Type C-] to exit the debugger.
  Type M-x, then press F5 => the debugger doesn't start, although the
    message appears that should have triggered the debugger.

The problem with dabbrev-expand and Tramp is similar, in that it stems
from the fact that 'message' leaves the text in the echo area until
the next keypress, whereas 'minibuffer-message' actively removes the
message before returning (instead of letting someone else remove it
when needed).

I think the more general underlying problem here is that
'minibuffer-message' behaves very differently from 'message'.  What we
need is to keep the original behavior of 'message', but teach it not
to overwrite the minibuffer contents under certain conditions.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> We already have 2 bug reports about it for 2 separate
> packages, and we cannot leave them unsolved.  I'm sure this is just
> the tip of a very large iceberg.

Both were for the same thing, though, and was fixed by Juri's patch.

> How do you propose we move forward?  You think we should install the
> timer proposed by Juri instead?

I think that looked like the right solution.

> We are developing Emacs 27 for more than 1.5 years.  We need to start
> its release cycle very soon, and we should start from a stable code
> base.

I agree that the timing for this change was unfortunate, especially for
such a low-level change.  It does make sense to me to back out the
change for now and then apply it to master again once the emacs-27
branch has been cut.  (Or perhaps doing it the other way around is
easier -- cut the emacs-27 branch and then revert the change there.)

I think you're right in that there may be other interactions that we
still don't have a full overview of.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I agree that the timing for this change was unfortunate, especially for
> such a low-level change.  It does make sense to me to back out the
> change for now and then apply it to master again once the emacs-27
> branch has been cut.

(But I want to emphasise that I think Juri's patch is the right
long-term solution -- it's been great the past few weeks not having
anything, at random, hide what I'm working on in the minibuffer.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 38457@debbugs.gnu.org,  stephen.berman@gmx.net,  Richard Stallman
>  <rms@gnu.org>,  juri@linkov.net
> Date: Wed, 11 Dec 2019 08:35:30 +0100
> 
> (But I want to emphasise that I think Juri's patch is the right
> long-term solution -- it's been great the past few weeks not having
> anything, at random, hide what I'm working on in the minibuffer.)

I think the solution I proposed later, to insert the minibuffer
contents before the message text, is a more correct long-term
solution, since it has a much more narrow implications, and doesn't
change the behavior of such low-level pervasive APIs.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: juri@linkov.net,  38457@debbugs.gnu.org,  stephen.berman@gmx.net,
>   Richard Stallman <rms@gnu.org>
> Date: Wed, 11 Dec 2019 08:32:44 +0100
> 
> I agree that the timing for this change was unfortunate, especially for
> such a low-level change.  It does make sense to me to back out the
> change for now and then apply it to master again once the emacs-27
> branch has been cut.  (Or perhaps doing it the other way around is
> easier -- cut the emacs-27 branch and then revert the change there.)
> 
> I think you're right in that there may be other interactions that we
> still don't have a full overview of.

Do you have an opinion on my later suggestion, to insert the
minibuffer contents before the message text, when 'message' is called
with a minibuffer being active?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Do you have an opinion on my later suggestion, to insert the
> minibuffer contents before the message text, when 'message' is called
> with a minibuffer being active?

Sorry; I missed that one.  Hm...  That does immediately sound like an
even better solution, but I'd have to see it in action first.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: juri@linkov.net,  38457@debbugs.gnu.org,  stephen.berman@gmx.net,
>   rms@gnu.org
> Date: Tue, 24 Dec 2019 17:30:58 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Do you have an opinion on my later suggestion, to insert the
> > minibuffer contents before the message text, when 'message' is called
> > with a minibuffer being active?
> 
> Sorry; I missed that one.  Hm...  That does immediately sound like an
> even better solution, but I'd have to see it in action first.

Not exactly what I suggested, but something similar is already in
action (both master and emacs-27 branches), courtesy of Juri.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> No, it was not only about y-or-n-p, but about any command that uses the minibuffer.
>
> As it turns out, not any command, because "M-x" and "C-x C-f" also use
> the minibuffer.
>
> And the original bug reports definitely were about y-or-n-p.

bug#34614 was not about y-or-n-p.  It was about any command that uses
the minibuffer.

> I don't see how this is a "hack" when it uses the same technique as
> your changes in 'message': checking a variable that is bound by other
> functions.  The advantage of my proposal is that it makes the new
> functionality opt-in, so that any commands which need this could have
> it by simply binding a variable, and would otherwise maintain its old
> behavior, which was there for eons.

Such variable already exists.  It's called message-in-echo-area.
You can enable it in the release branch if you want.
But then please reopen bug#34614, bug#19064, bug#17272, bug#446.

> Here's one more problem with the new behavior of 'message':
>
>   emacs -Q
>
>   Evaluate:
>
>   (defun my-mesage ()
>     (interactive)
>     (message "my most important message"))
>   (global-set-key [f5] 'my-mesage)
>   (setq debug-on-message "important")
>
>   Press F5, observe the debugger pop up, as expected.
>   Type C-] to exit the debugger.
>   Type M-x, then press F5 => the debugger doesn't start, although the
>     message appears that should have triggered the debugger.

This is exactly the purpose of the pretest - you are testing a new feature
or a bug fix, then discover that some feature doesn't work, report it,
and the following patch implements the missing feature.

Looking at the recent log, there are many fixes in core functions
with potentially destabilizing changes still committed every day.
How fixes in minibuffer-message are different from other
more risky fixes in other core functions?

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 8af8aca30e..125a711b54 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -749,7 +749,11 @@ minibuffer-message
               (put-text-property 0 1 'cursor t message))
             (overlay-put ol 'after-string message)
             (sit-for (or minibuffer-message-timeout 1000000)))
-        (delete-overlay ol)))))
+        (delete-overlay ol)))
+    (when (and (stringp debug-on-message)
+               (stringp message)
+               (string-match-p debug-on-message message))
+      (debug message))))
 
 (defun minibuffer-completion-contents ()
   "Return the user input in a minibuffer before point as a string.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org,  stephen.berman@gmx.net
> Date: Tue, 10 Dec 2019 01:45:18 +0200
> 
> > And the original bug reports definitely were about y-or-n-p.
> 
> bug#34614 was not about y-or-n-p.  It was about any command that uses
> the minibuffer.

I was talking about the 3 bug reports mentioned in the change's log.

> > I don't see how this is a "hack" when it uses the same technique as
> > your changes in 'message': checking a variable that is bound by other
> > functions.  The advantage of my proposal is that it makes the new
> > functionality opt-in, so that any commands which need this could have
> > it by simply binding a variable, and would otherwise maintain its old
> > behavior, which was there for eons.
> 
> Such variable already exists.  It's called message-in-echo-area.
> You can enable it in the release branch if you want.
> But then please reopen bug#34614, bug#19064, bug#17272, bug#446.

Sorry, I don't understand the proposal.  How will this variable help
if we leave the current code in 'message' as it is?  And what do you
mean by "enabling" message-in-echo-area?

> >   Type M-x, then press F5 => the debugger doesn't start, although the
> >     message appears that should have triggered the debugger.
> 
> This is exactly the purpose of the pretest - you are testing a new feature
> or a bug fix, then discover that some feature doesn't work, report it,
> and the following patch implements the missing feature.

I expect to see a lot of such problems, and consequently a lot of
patches.  More importantly, I expect quite a few of those to slip into
the release.  That's because minibuffer-message's behavior is very
different from that of 'message', and these differences will bite us
every turn for a long time.

This is not the right way of fixing the problems with overwriting the
prompts by messages.  It will certainly prolong the pretest too much,
and most probably leave some problems undiscovered and unsolved.

> Looking at the recent log, there are many fixes in core functions
> with potentially destabilizing changes still committed every day.
> How fixes in minibuffer-message are different from other
> more risky fixes in other core functions?

They are much more risky because they are in an infrastructure used
all over the place, and also because the method selected for
displaying such messages by minibuffer-message changes the behavior in
very significant ways, so it will certainly cause many unintended
consequences.

The patch below also changes the behavior of minibuffer-message wrt
debug-on-message, doesn't it?  If so, we cannot install it as shown.

We must find a better solution for the original problems, or decide to
postpone the solution till after Emacs 27.  Please help me in this
matter.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 3470 bytes --]

I reported the bug #34614.

I meet the bug(#34614 like) everyday. One common scene: I push a commit with magit. And then open a file with C-xC-f, a message (“git finished.”) comes and replace the find-file prompt. I need to press C-g and C-xC-f again.

This is not a big problem. But minibuffer is a basic UI of emacs. If it always has these bugs, users may think that minibuffer is not a good design.

在 2019年12月10日 +0800 AM11:37，Eli Zaretskii <eliz@gnu.org>，写道：
> > From: Juri Linkov <juri@linkov.net>
> > Cc: 38457@debbugs.gnu.org, stephen.berman@gmx.net
> > Date: Tue, 10 Dec 2019 01:45:18 +0200
> >
> > > And the original bug reports definitely were about y-or-n-p.
> >
> > bug#34614 was not about y-or-n-p. It was about any command that uses
> > the minibuffer.
>
> I was talking about the 3 bug reports mentioned in the change's log.
>
> > > I don't see how this is a "hack" when it uses the same technique as
> > > your changes in 'message': checking a variable that is bound by other
> > > functions. The advantage of my proposal is that it makes the new
> > > functionality opt-in, so that any commands which need this could have
> > > it by simply binding a variable, and would otherwise maintain its old
> > > behavior, which was there for eons.
> >
> > Such variable already exists. It's called message-in-echo-area.
> > You can enable it in the release branch if you want.
> > But then please reopen bug#34614, bug#19064, bug#17272, bug#446.
>
> Sorry, I don't understand the proposal. How will this variable help
> if we leave the current code in 'message' as it is? And what do you
> mean by "enabling" message-in-echo-area?
>
> > > Type M-x, then press F5 => the debugger doesn't start, although the
> > > message appears that should have triggered the debugger.
> >
> > This is exactly the purpose of the pretest - you are testing a new feature
> > or a bug fix, then discover that some feature doesn't work, report it,
> > and the following patch implements the missing feature.
>
> I expect to see a lot of such problems, and consequently a lot of
> patches. More importantly, I expect quite a few of those to slip into
> the release. That's because minibuffer-message's behavior is very
> different from that of 'message', and these differences will bite us
> every turn for a long time.
>
> This is not the right way of fixing the problems with overwriting the
> prompts by messages. It will certainly prolong the pretest too much,
> and most probably leave some problems undiscovered and unsolved.
>
> > Looking at the recent log, there are many fixes in core functions
> > with potentially destabilizing changes still committed every day.
> > How fixes in minibuffer-message are different from other
> > more risky fixes in other core functions?
>
> They are much more risky because they are in an infrastructure used
> all over the place, and also because the method selected for
> displaying such messages by minibuffer-message changes the behavior in
> very significant ways, so it will certainly cause many unintended
> consequences.
>
> The patch below also changes the behavior of minibuffer-message wrt
> debug-on-message, doesn't it? If so, we cannot install it as shown.
>
> We must find a better solution for the original problems, or decide to
> postpone the solution till after Emacs 27. Please help me in this
> matter.
>
> Thanks.
>
>
>

[-- Attachment #2: Type: text/html, Size: 5276 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Tue, 10 Dec 2019 15:19:37 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: 38457@debbugs.gnu.org, stephen.berman@gmx.net
> 
> I reported the bug #34614.
> 
> I meet the bug(#34614 like) everyday. One common scene: I push a commit with magit. And then open a file
> with C-xC-f, a message (“git finished.”) comes and replace the find-file prompt. I need to press C-g and
> C-xC-f again.

There's no need for something as drastic as C-g.  You could instead
press left-arrow, for example, or some other similarly harmless key.

> This is not a big problem. But minibuffer is a basic UI of emacs. If it always has these bugs, users may think
> that minibuffer is not a good design. 

No one argues against fixing this annoyance.  But the fix should not
cause more problems than the problem it fixes, and it should not
change behavior in incompatible ways.  In addition, we want to start
the Emacs 27 release cycle soon, so we don't want invasive changes in
infrastructure used all over the place.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Drew Adams]

> There's no need for something as drastic as C-g.  You could instead
> press left-arrow, for example, or some other similarly harmless key.

Yes.  It might be good to (somewhere) point
this out in the Emacs manual.  Some aspects
of (useful) minibuffer interaction might not
be obvious.

> No one argues against fixing this annoyance.  But the fix should not
> cause more problems than the problem it fixes, and it should not
> change behavior in incompatible ways. In addition, we want to start
> the Emacs 27 release cycle soon, so we don't want invasive changes in
> infrastructure used all over the place.

+1

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Drew Adams]

> This is not a big problem. But minibuffer is a
> basic UI of emacs. If it always has these bugs, 
> users may think that minibuffer is not a good 
> design.

The minibuffer does _not_ "have these bugs".
The problem is not the minibuffer.

The problems are (1) asynchronous reporting
that simply fires off a `message' blindly and
(2) read-key interaction misused for important,
essentially modal dialog, which, if during a
minibuffer interaction, can be problematic.

Emacs should provide other, safer ways to
report status or impose a separate, modal
dialog - ways that don't interfere with the
minibuffer or the echo area.

`message-box' and similar might be a start.
But the behavior should be loggable, and it
should be manipulable by program in various
ways (prompting, whether modal, etc.), and
it should be customizable by users.

And developers should then be encouraged to
change any existing code that provides async
reporting or intermediate `y-or-n-p'-style
dialogs to use the new constructs.

At least those who use such things should be
made aware that if they interrupt ongoing
interactions then they can cause trouble.
___

And for the record, I disagree with simply
automatically converting `message' behavior
to `minibuffer-message' behavior in any
systematic way.

And I disagree with replacing `read-key'
behavior for code like `y-or-n-p' with
some read-from-the-minibuffer behavior in
any systematic way.

Some given uses of these things might be
misguided and could be switched to using
`minibuffer-message' - i.e., case by case.
I'm not against `minibuffer-message' - it
has its uses.  But its uses mainly involve
providing feedback about the ongoing
minibuffer interaction - not reporting an
outside event.

One of the demonstrations of the "dangerous
problem" was from Juri changing the value of
variable `dired-deletion-confirmer' (which
is _not_ a user option) from `yes-or-no-p'
to `y-or-n-p', and then pointing to a
resulting possible loss of user data.

Well, duh - don't do that.  That variable is
there presumably for possible use by code in
a context where the deletion dialog is sure,
and controlled in some way.

Many users seem to have chosen to just
replace `yes-or-no-p' with `y-or-n-p, e.g.
by aliasing.  I think this is a practice
that should be discouraged, or at least
shouldn't be encouraged.  Code that uses
`yes-or-no-p' should be able to depend on
its response not getting lost.  Blindly
substituting `y-or-n-p' is a bad idea, IMO.

Like the problem of async reporting, reading
a key/char can be problematic.  Emacs can
hopefully come up with additional ways,
better in some contexts, to prompt for a key
that reduce or eliminate the problem.  But
foregoing quick key/char reading by forcing
use of the minibuffer is a bad "fix".

Yes, there is a _real_ problem to be solved.
There may even be more than one problem.
(Async reporting is not the same as reading
a key/char.  The fixes are not necessarily
the same.)

But no, the one-size-fits-all approach taken
so far is not a good solution, IMO.

It screws the minibuffer - an editable
buffer allowing complex user interactions.

It screws with the difference, when using
the minibuffer, between (1) the behavior of
`message' (use the echo area, temporal
interruption dismissable by typing) and (2)
that of `minibuffer-message' (append output
to the minibuffer input, no interruption -
interfere spatially, not temporally).

Each of those, minibuffer-message' and
`message', has its particular uses and
behavior while the minibuffer is active.
And the various key/char-reading functions
also have their particular uses.  None of
these should be tossed or stifled.

Throwing all that to the wind, neutering
it with a one-size-fits-all behavior, is a
step backward.  We should fix the various
problems carefully - very carefully, and
in context.

Don't start with the supposition that the
minibuffer is the problem.  It's code that
interferes with a minibuffer interaction
that's the problem.

Whether, when, and how to interrupt an
existing dialog is the question.  The
answer is, "it depends".

Emacs should be able to let a minibuffer
interaction coexist safely with async
reporting or key/char reading - without
just stuffing in timeouts and tossing
everything into `minibuffer-message'.

In some cases an interaction (user dialog)
may need to be _modal_ - finish reading a
confirmation before allowing other action.
This is about such an interaction coming
about while another, minibuffer, dialog
already exists.

In other cases that's not necessary.  The
calling code that prompts for what should
be a modal interaction should DTRT.  It's
not up to another, minibuffer, interaction
to protect against such disruption.
___

Just one opinion - expressed in more detail
in this and some other bug threads.

The fact that this "fix" is spread over
several bug threads, dealing with different 
problematic behaviors, is perhaps a sign
that this approach is not TRT.

And such a far-reaching change should, I
think, have been discussed in emacs-devel.

Instead, in these bug threads we've been
told that this is only a minor, localized
change that will not affect 3rd-party code.
That does _not_ seem to be the case, and I
don't see how it could be.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Tue, 10 Dec 2019 05:36:24 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 38457@debbugs.gnu.org, stephen.berman@gmx.net
> 
> > > I don't see how this is a "hack" when it uses the same technique as
> > > your changes in 'message': checking a variable that is bound by other
> > > functions.  The advantage of my proposal is that it makes the new
> > > functionality opt-in, so that any commands which need this could have
> > > it by simply binding a variable, and would otherwise maintain its old
> > > behavior, which was there for eons.
> > 
> > Such variable already exists.  It's called message-in-echo-area.
> > You can enable it in the release branch if you want.
> > But then please reopen bug#34614, bug#19064, bug#17272, bug#446.
> 
> Sorry, I don't understand the proposal.  How will this variable help
> if we leave the current code in 'message' as it is?  And what do you
> mean by "enabling" message-in-echo-area?

Actually, the more I think about this the less I like the idea of
calling minibuffer-message instead of 'message' under some
circumstances, any circumstances.  minibuffer-message is meant for
very different use case: temporarily displaying a message for a short
time.  The messages it displays are usually ephemeral and dispensable;
if the user misses such a message, no big deal.

By contrast, 'message' is used for similar use cases, but also for
radically different ones, including those where several related
messages are displayed in quick succession (a notable example is
"Foo..." followed by "Foo...done"), or where a message is left in the
echo area indefinitely if no input event arrives.  Crucially, Lisp
programs do not tell 'message' which use case is required; instead,
the following calls to 'message' or other events produce the required
effects.  To produce the same effect with minibuffer-message will
require too many changes all over the place.

So I think it is simply wrong to call minibuffer-message instead of
'message'.  There are too many different behavior aspects.  Several
examples were already given, including debug-on-message.  One other
aspect that I just bumped into is recording messages in the *Messages*
buffer: minibuffer-message never did that, until a recent (and
undocumented) change, also related to attempts to fix messages
overwriting y-or-n-p prompts, so now we have one more incompatible
change in minibuffer-message's behavior waiting to bite us down the
road.

Therefore, I suggest to take a step back and discuss a better solution
for these problems.

The original problem was, AFAIU, that various minibuffer prompts
become obscured by echo-area display of messages.  So one possible
solution is to modify the subroutines of 'message', e.g.,
set_message_1, to detect the conditions of the minibuffer being
active, and insert the contents of the minibuffer into the echo-area
buffer before the message text.  Does anyone see problems with this?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

> The original problem was, AFAIU, that various minibuffer prompts
> become obscured by echo-area display of messages.  So one possible
> solution is to modify the subroutines of 'message', e.g.,
> set_message_1, to detect the conditions of the minibuffer being
> active, and insert the contents of the minibuffer into the echo-area
> buffer before the message text.

Ha!  Clever!

> Does anyone see problems with this?

[ I presume that additionally from inserting the minibuffer's contents
  we'd add some delimiters to clearly separate the message from the
  minibuffer's contents.  ]

I guess there could be some visible artifacts of this subterfuge in the
case where the minibuffer's content is currently affected by overlays
(since we presumably wouldn't copy those to the echo-area), but I'm not
sure this would be a real problem.

One such case of overlays is when icomplete-mode is enabled, and it
would simply temporarily hide the completion-list displayed by
icomplete-mode, which seems quite harmless or maybe even beneficial.

Another issue could be when the minibuffer is large, in which case we'll
want to try and preserve the window-start but also we'll want to make
sure the new message is visible.

BTW, we should probably think about replacing `message` with something
that lets the caller give more information about the intended behavior
(e.g. to also solve the issue of several successive calls to message
resulting in the user only seeing the last message).  Of course, this
would be for Emacs-28.


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juri@linkov.net,  38457@debbugs.gnu.org
> Date: Tue, 10 Dec 2019 12:08:14 -0500
> 
> [ I presume that additionally from inserting the minibuffer's contents
>   we'd add some delimiters to clearly separate the message from the
>   minibuffer's contents.  ]

Something like minibuffer-message's "[..]", perhaps?

> I guess there could be some visible artifacts of this subterfuge in the
> case where the minibuffer's content is currently affected by overlays
> (since we presumably wouldn't copy those to the echo-area), but I'm not
> sure this would be a real problem.

We could copy the overlays, if that's important.

> Another issue could be when the minibuffer is large, in which case we'll
> want to try and preserve the window-start but also we'll want to make
> sure the new message is visible.

You mean, if resize-mini-windows is nil?  Otherwise, I see no problem.

If resize-mini-windows is nil, the result will be the same as with the
current code, which calls minibuffer-message: the message is partially
or completely invisible.

> BTW, we should probably think about replacing `message` with something
> that lets the caller give more information about the intended behavior
> (e.g. to also solve the issue of several successive calls to message
> resulting in the user only seeing the last message).  Of course, this
> would be for Emacs-28.

Right.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Tue, 10 Dec 2019 19:49:00 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 38457@debbugs.gnu.org, juri@linkov.net
> 
> > I guess there could be some visible artifacts of this subterfuge in the
> > case where the minibuffer's content is currently affected by overlays
> > (since we presumably wouldn't copy those to the echo-area), but I'm not
> > sure this would be a real problem.
> 
> We could copy the overlays, if that's important.

Btw, the current code on master also removes the overlays where
icomplete shows the completion candidates.  You can try it:

  . Evaluate:

    (defun my-message ()
      (interactive)
      (message "my most important message"))
    (global-set-key [f5] 'my-message)

  . M-x icomplete-mode RET
  . C-x C-f
  . F5

After "C-x C-f" the list of candidates will be displayed in the
minibuffer; pressing F5 removes the list and shows the message
instead.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> Btw, the current code on master also removes the overlays where
> icomplete shows the completion candidates.  You can try it:
>
>   . Evaluate:
>
>     (defun my-message ()
>       (interactive)
>       (message "my most important message"))
>     (global-set-key [f5] 'my-message)
>
>   . M-x icomplete-mode RET
>   . C-x C-f
>   . F5
>
> After "C-x C-f" the list of candidates will be displayed in the
> minibuffer; pressing F5 removes the list and shows the message
> instead.

That's not fair.  I fixed this already a week ago.
But you didn't allow me to install the fix.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  38457@debbugs.gnu.org
> Date: Wed, 11 Dec 2019 01:45:30 +0200
> 
> > Btw, the current code on master also removes the overlays where
> > icomplete shows the completion candidates.  You can try it:
> >
> >   . Evaluate:
> >
> >     (defun my-message ()
> >       (interactive)
> >       (message "my most important message"))
> >     (global-set-key [f5] 'my-message)
> >
> >   . M-x icomplete-mode RET
> >   . C-x C-f
> >   . F5
> >
> > After "C-x C-f" the list of candidates will be displayed in the
> > minibuffer; pressing F5 removes the list and shows the message
> > instead.
> 
> That's not fair.  I fixed this already a week ago.
> But you didn't allow me to install the fix.

Sorry, I don't think I follow: what did you want to fix in this
scenario?

I'm guessing this is about the timer you proposed that would remove
the text shown by minibuffer-message.  If so, my point above was not
about the time it takes to remove that text with the current master,
my point was that while the message is displayed, the icomplete-mode
candidates are not displayed.  (FWIW, I don't consider this a bug we
should fix, and neither does Stefan, AFAIU.)

Am I missing something?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

> my point was that while the message is displayed, the icomplete-mode
> candidates are not displayed.  (FWIW, I don't consider this a bug we
> should fix, and neither does Stefan, AFAIU.)

You understood correctly: I do not consider this a bug (tho it might be
purely accidental, as I suspect it is: it's likely the result of the
fact that the list of completion is removed by icomplete before
processing a command and re-added afterwards, and minibuffer-message
adds a 2s wait during the execution of the command, i.e. at a point in
time where icomplete's list is not displayed).


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Juri Linkov <juri@linkov.net>,  38457@debbugs.gnu.org
> Date: Wed, 11 Dec 2019 12:34:26 -0500
> 
> You understood correctly: I do not consider this a bug (tho it might be
> purely accidental, as I suspect it is: it's likely the result of the
> fact that the list of completion is removed by icomplete before
> processing a command and re-added afterwards, and minibuffer-message
> adds a 2s wait during the execution of the command, i.e. at a point in
> time where icomplete's list is not displayed).

I actually thought this was the result of adding an overlay at the
end, which "covers" the icomplete's overlay.  But I didn't look
closely at the code, so maybe I'm wrong.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

>> You understood correctly: I do not consider this a bug (tho it might be
>> purely accidental, as I suspect it is: it's likely the result of the
>> fact that the list of completion is removed by icomplete before
>> processing a command and re-added afterwards, and minibuffer-message
>> adds a 2s wait during the execution of the command, i.e. at a point in
>> time where icomplete's list is not displayed).
>
> I actually thought this was the result of adding an overlay at the
> end, which "covers" the icomplete's overlay.  But I didn't look
> closely at the code, so maybe I'm wrong.

Could be as well, I don't actually know, the above is just my
best guess.  In any case I think it's not a bug (not sure if I'd go so
far as to call it a feature, tho).


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > Btw, the current code on master also removes the overlays where
>> > icomplete shows the completion candidates.  You can try it:
>> >
>> >   . Evaluate:
>> >
>> >     (defun my-message ()
>> >       (interactive)
>> >       (message "my most important message"))
>> >     (global-set-key [f5] 'my-message)
>> >
>> >   . M-x icomplete-mode RET
>> >   . C-x C-f
>> >   . F5
>> >
>> > After "C-x C-f" the list of candidates will be displayed in the
>> > minibuffer; pressing F5 removes the list and shows the message
>> > instead.
>>
>> That's not fair.  I fixed this already a week ago.
>> But you didn't allow me to install the fix.
>
> Sorry, I don't think I follow: what did you want to fix in this
> scenario?

I already fixed this 10 days ago.  If the patch was installed then
you wouldn't say that the current code on master removes the icomplete
overlays.  Because the patch doesn't remove the icomplete overlays.
Its behavior is so smooth that it would be unfortunate to implement
something worse than that.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  38457@debbugs.gnu.org
> Date: Thu, 12 Dec 2019 01:12:33 +0200
> 
> > Sorry, I don't think I follow: what did you want to fix in this
> > scenario?
> 
> I already fixed this 10 days ago.  If the patch was installed then
> you wouldn't say that the current code on master removes the icomplete
> overlays.  Because the patch doesn't remove the icomplete overlays.
> Its behavior is so smooth that it would be unfortunate to implement
> something worse than that.

By "the patch" do you mean the patch that introduced a timer to remove
the minibuffer message?

If so, I don't understand how would it change the scenario with
icomplete, please elaborate.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > Sorry, I don't think I follow: what did you want to fix in this
>> > scenario?
>>
>> I already fixed this 10 days ago.  If the patch was installed then
>> you wouldn't say that the current code on master removes the icomplete
>> overlays.  Because the patch doesn't remove the icomplete overlays.
>> Its behavior is so smooth that it would be unfortunate to implement
>> something worse than that.
>
> By "the patch" do you mean the patch that introduced a timer to remove
> the minibuffer message?

Exactly.

> If so, I don't understand how would it change the scenario with
> icomplete, please elaborate.

Have you tried that patch in action?

I think it works perfectly well.  But if you don't agree then it's fine
to enable it optionally.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  38457@debbugs.gnu.org
> Date: Fri, 13 Dec 2019 00:45:56 +0200
> 
> >> > Sorry, I don't think I follow: what did you want to fix in this
> >> > scenario?
> >>
> >> I already fixed this 10 days ago.  If the patch was installed then
> >> you wouldn't say that the current code on master removes the icomplete
> >> overlays.  Because the patch doesn't remove the icomplete overlays.
> >> Its behavior is so smooth that it would be unfortunate to implement
> >> something worse than that.
> >
> > By "the patch" do you mean the patch that introduced a timer to remove
> > the minibuffer message?
> 
> Exactly.
> 
> > If so, I don't understand how would it change the scenario with
> > icomplete, please elaborate.
> 
> Have you tried that patch in action?
> 
> I think it works perfectly well.  But if you don't agree then it's fine
> to enable it optionally.

Sorry, I cannot afford trying it: too much on my plate right now.
Please humor me by explaining what kind of change in behavior it
produces in this scenario.

TIA.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

>> I guess there could be some visible artifacts of this subterfuge in the
>> case where the minibuffer's content is currently affected by overlays
>> (since we presumably wouldn't copy those to the echo-area), but I'm not
>> sure this would be a real problem.
> We could copy the overlays, if that's important.

My gut feeling is that it's not important.

>> Another issue could be when the minibuffer is large, in which case we'll
>> want to try and preserve the window-start but also we'll want to make
>> sure the new message is visible.
> You mean, if resize-mini-windows is nil?

In any case where the window-start isn't point-min.
That can be due to resize-mini-windows being nil or any other reason.

> Otherwise, I see no problem.

I'm not terribly worried about it either.
I just mentioned it as a problem I could imagine coming up.

> If resize-mini-windows is nil, the result will be the same as with the
> current code, which calls minibuffer-message: the message is partially
> or completely invisible.

With the current code (i.e. with minibuffer-message), if
resize-mini-windows is nil and the minibuffer is large enough to matter,
the message is still (partially) visible as long as the currently
displayed part of the minibuffer includes EOB, which is the usual case.

If we simply copy the minibuffer's content to the echo area without
preserving window-start, it will be more disruptive and will make it more
likely that the message isn't visible.


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: juri@linkov.net,  38457@debbugs.gnu.org
> Date: Tue, 10 Dec 2019 14:45:35 -0500
> 
> With the current code (i.e. with minibuffer-message), if
> resize-mini-windows is nil and the minibuffer is large enough to matter,
> the message is still (partially) visible as long as the currently
> displayed part of the minibuffer includes EOB, which is the usual case.
> 
> If we simply copy the minibuffer's content to the echo area without
> preserving window-start, it will be more disruptive and will make it more
> likely that the message isn't visible.

There's some misunderstanding here: what I had in mind was to insert
the minibuffer contents into the echo-area buffer, put the point after
that, and then insert the text which 'message' was called to display.
With this, window-start will be at point-min of the echo-area buffer,
or am I missing something?

IOW, I don't understand why you are worried about window-start, since
it should always be at point-min, and the mini-window should resize to
show all the text.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

>> With the current code (i.e. with minibuffer-message), if
>> resize-mini-windows is nil and the minibuffer is large enough to matter,
>> the message is still (partially) visible as long as the currently
>> displayed part of the minibuffer includes EOB, which is the usual case.
>> 
>> If we simply copy the minibuffer's content to the echo area without
>> preserving window-start, it will be more disruptive and will make it more
>> likely that the message isn't visible.
>
> There's some misunderstanding here: what I had in mind was to insert
> the minibuffer contents into the echo-area buffer, put the point after
> that, and then insert the text which 'message' was called to display.

Then I guess window-start may be changed by the redisplay to keep point
displayed, so I think that'll work fine.

> IOW, I don't understand why you are worried about window-start, since
> it should always be at point-min, and the mini-window should resize to
> show all the text.

The above was under the (explicit) assumption of resize-mini-windows
being nil.  There can be other cases where window-start isn't point-min
(e.g. if the minibuffer is too large to fit in the max size of the mini
window).

But no, I'm not worried about it ;-)


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

在 2019年12月11日 +0800 AM1:50，Eli Zaretskii <eliz@gnu.org>，写道：
>
> If resize-mini-windows is nil, the result will be the same as with the
> current code, which calls minibuffer-message: the message is partially
> or completely invisible.
>
> > BTW, we should probably think about replacing `message` with something
> > that lets the caller give more information about the intended behavior
> > (e.g. to also solve the issue of several successive calls to message
> > resulting in the user only seeing the last message). Of course, this
> > would be for Emacs-28.
>
> Right.
>

Display combination of the prompt and the message is good but may be complex to implement it.
What about display message transiently and then restore to prompt?

I see the doc string of minibuffer-message-timeout says:

	How long to display an echo-area message when the minibuffer is active.
If the value is a number, it should be specified in seconds.
If the value is not a number, such messages never time out.

If minibuffer is active, the message will be displayed  for a moment(like 1s) and then disappears.
If minibuffer is not active, the message will be displayed forever. I think this is simpler and also good for me.

What is the problem of this behaviour? Is it easier to implement?



[-- Attachment #2: Type: text/html, Size: 2301 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Wed, 11 Dec 2019 11:24:37 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: 38457@debbugs.gnu.org, juri@linkov.net
> 
> Display combination of the prompt and the message is good but may be complex to implement it.

I explained in more detail how I suggested to implement thius: by
inserting the minibuffer contents before the message text.  What is
complex about that?

> What about display message transiently and then restore to prompt?
> 
> I see the doc string of minibuffer-message-timeout says: 
> 
>  How long to display an echo-area message when the minibuffer is active.
>  If the value is a number, it should be specified in seconds.
>  If the value is not a number, such messages never time out.
> 
> If minibuffer is active, the message will be displayed  for a moment(like 1s) and then disappears.
> If minibuffer is not active, the message will be displayed forever. I think this is simpler and also good for me.
> 
> What is the problem of this behaviour?

This is what the current code does, and the problem with that is that
some uses of 'message' don't expect the message to stay for 2 seconds,
and some expect it to stay forever.  This information is not explicit
in the call to 'message', so there's no way of communicating in down
to minibuffer-message.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

在 2019年12月11日 +0800 AM11:40，Eli Zaretskii <eliz@gnu.org>，写道：
> I explained in more detail how I suggested to implement thius: by
> inserting the minibuffer contents before the message text. What is
> complex about that?
>

Because resize-mini-windows may be nil. And the message may be invisible.

Another question:
When emacs displays combination of the prompt and the message, if user input something, doesn’t the message disappears?

>
> This is what the current code does,

The current code doesn’t display the message transiently. It displays it forever. User needs to press a key to restore to the prompt. Is this a bug?

> and the problem with that is that
> some uses of 'message' don't expect the message to stay for 2 seconds,
> and some expect it to stay forever. This information is not explicit
> in the call to 'message', so there's no way of communicating in down
> to minibuffer-message.

Do you mean the scene “Foo…” followed by “Foo…done”?
I don’t think this is a problem. These are just status which are not important. And they disappears quickly when user is busy editing. And the really important ones may put an indicator on the mode-line.





[-- Attachment #2: Type: text/html, Size: 2365 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Wed, 11 Dec 2019 11:59:48 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: monnier@iro.umontreal.ca, 38457@debbugs.gnu.org, juri@linkov.net
> 
>  I explained in more detail how I suggested to implement thius: by
>  inserting the minibuffer contents before the message text. What is
>  complex about that?
> 
> Because resize-mini-windows may be nil. And the message may be invisible. 

The current master has the same problem.

In general, people who set resize-mini-windows to nil are always in
danger of seeing only part of the message Emacs displays.

> Another question: 
> When emacs displays combination of the prompt and the message, if user input something, doesn’t the
> message disappears?

It does disappear, and the prompt remains.  Again, lime with the
current master.

>  This is what the current code does,  
> 
> The current code doesn’t display the message transiently. It displays it forever.

By "the current code" I meant the current master branch.  There, the
message is displayed for 2 sec, and then disappears, even if the user
didn't press any key.  Which is different from how 'message' behaves
when there's no prompt.

> User needs to press a key to
> restore to the prompt. Is this a bug?

No, I don't think it's a bug.  If some Lisp program displays a message
and doesn't follow it with a nil message, it means that Lisp program
_wants_ the message to remain on display until the user dismisses it.

>  and the problem with that is that
>  some uses of 'message' don't expect the message to stay for 2 seconds,
>  and some expect it to stay forever. This information is not explicit
>  in the call to 'message', so there's no way of communicating in down
>  to minibuffer-message. 
> 
> Do you mean the scene “Foo…” followed by “Foo…done”?

That's one scenario, yes.  But it is not the only one.

> I don’t think this is a problem. These are just status which are not important. And they disappears quickly
> when user is busy editing.

With the current master, it doesn't disappear quickly, it stays for 2 sec.

> And the really important ones may put an indicator on the mode-line.

What indicator?

(And I don't understand why we are arguing, since you just said in
another message that you liked my proposal...)

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 4220 bytes --]

在 2019年12月12日 +0800 AM12:26，Eli Zaretskii <eliz@gnu.org>，写道：
> In general, people who set resize-mini-windows to nil are always in
> danger of seeing only part of the message Emacs displays.
>

For the combination of prompt and message, there is a worst case: the message may never be displayed because no space in minibuffer window. The user may never notice it.


> > Another question:
> > When emacs displays combination of the prompt and the message, if user input something, doesn’t the
> > message disappears?
>
> It does disappear, and the prompt remains. Again, lime with the
> current master.
>

When user is inputing something in minibuffer, I think there is hardly any message so important to let user pause the inputing and wait for the next message, except they are BBC news.

The importance levels for me:

    (1) Inputing in minibuffer
    (2) Inputing in buffer window
    (3) Messages in minibuffer window

I think messages(In (3)) should not disturb (1). Because they are so less important.
And when user is busy editing in (2), he may never notice messages in minibuffer because they are cleared quickly. So important messages should not use minibuffer. They should use mode-line or something else.

One example for using mode-line:
Magit is a git tool in emacs. When push with magit, it display “push” in mode-line and clear it when done.


> > This is what the current code does,
> >
> > The current code doesn’t display the message transiently. It displays it forever.
>
> By "the current code" I meant the current master branch.

I meant the code before minibuffer-message is added.


> There, the
> message is displayed for 2 sec, and then disappears, even if the user
> didn't press any key. Which is different from how 'message' behaves
> when there’s no prompt.

I like this behaviour. There are two sub-behaviours:

    (1) Display the combination of prompt and message. After 2s, display prompt only
    (2) Display message only. After 2s, back to display prompt only.

The problem(for UI only) with (1) is that there is a worst case mentioned above (when resize-mini-windows is nil). But it is well for almost 99%.

Both of them are OK to me. I just think that (2) may be more simple and may have less conflicts with other packages.

By the way, ivy (a popular package for emacs) uses (2). It works in minibuffer. When a message comes, it displays the message. After a moment, it clears the message and restores its UI.


>
> > User needs to press a key to
> > restore to the prompt. Is this a bug?
>
> No, I don't think it's a bug. If some Lisp program displays a message
> and doesn't follow it with a nil message, it means that Lisp program
> _wants_ the message to remain on display until the user dismisses it.
>
The current behaviour in emacs-26(and the master branch before minibuffer-message is added) is that emacs hides the prompt and displays the message forever. Most lisp programs don’t display the nil message. So the prompt will not be resotred automatically.

I think it is hard for lisp programs to display the nil message(It may have to setup a timer). So most of them don’t do this. I don’t know one of them.

And I think inputing in minibuffer or editing in buffer is much more important than these messages. If the message is too important, it should go to mode-line or else.


> What indicator?
>
See the example of magit above.

> (And I don't understand why we are arguing, since you just said in
> another message that you liked my proposal…)
Yes. I like your proposal that message do these things internally. Because the input prompt should always be considered, so all these things can be done in ‘message’. Then it seems that we don’t need another api(like minibuffer-message)?

Is the UI of your proposal like this?
(1) Display combination of the prompt and the message with no timeout.
(2) If user inputs something, the message is cleared and only prompt is displayed.

It is also simple. I like it.


The last question:
What is the use of minibuffer-message-timeout. It is defined in C. When does it take effect?



[-- Attachment #2: Type: text/html, Size: 7303 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Thu, 12 Dec 2019 12:33:09 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: monnier@iro.umontreal.ca, 38457@debbugs.gnu.org, juri@linkov.net
> 
>  (And I don't understand why we are arguing, since you just said in
>  another message that you liked my proposal…) 
> 
> Yes. I like your proposal that message do these things internally. Because the input prompt should always be
> considered, so all these things can be done in ‘message’. Then it seems that we don’t need another api(like
> minibuffer-message)?
> 
> Is the UI of your proposal like this?
> (1) Display combination of the prompt and the message with no timeout.
> (2) If user inputs something, the message is cleared and only prompt is displayed.

Yes.

> The last question: 
> What is the use of minibuffer-message-timeout. It is defined in C. When does it take effect?

It affects how long minibuffer-message waits before removing the
message text, if there are no input events.  Any input event removes
the message even if the timeout didn't expire.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> The importance levels for me:
>
>     (1) Inputing in minibuffer
>     (2) Inputing in buffer window
>     (3) Messages in minibuffer window
>
> I think messages(In (3)) should not disturb (1). Because they are so less important. 
> And when user is busy editing in (2), he may never notice messages in
> minibuffer because they are cleared quickly.  So important messages
> should not use minibuffer.  They should use mode-line or something else.

Really important messages should be displayed permanently in a pop-up window
like display-delayed-warnings and display-warning.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  monnier@iro.umontreal.ca,
>   38457@debbugs.gnu.org
> Date: Fri, 13 Dec 2019 00:58:15 +0200
> 
> Really important messages should be displayed permanently in a pop-up window
> like display-delayed-warnings and display-warning.

If you have such plans, I suggest to discuss them on emacs-devel
first.  The really hard nut, IMO, is to decide how we distinguish
really important messages from the rest, and how we communicate this
to 'message'.  In any case, I hope you agree this is not for Emacs 28.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Fri, 13 Dec 2019 09:02:09 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 38457@debbugs.gnu.org, netjune@outlook.com, monnier@iro.umontreal.ca
> 
> In any case, I hope you agree this is not for Emacs 28.

Sorry, I meant not for Emacs 27.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Stefan Monnier]

> In general, people who set resize-mini-windows to nil are always in
> danger of seeing only part of the message Emacs displays.

Actually, with your proposal of putting point right between the
minibuffer content and the message, while there is still a risk that
some of the message is not visible, you at least ensure that some part
of the message is visible, which is already better than what we
currently have (where the message is invisible if EOB is after
window-end).

I think it's pretty close to the best that can be done in this respect.


        Stefan

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

在 2019年12月11日 +0800 AM12:36，Eli Zaretskii <eliz@gnu.org>，写道：
> The original problem was, AFAIU, that various minibuffer prompts
> become obscured by echo-area display of messages. So one possible
> solution is to modify the subroutines of 'message', e.g.,
> set_message_1, to detect the conditions of the minibuffer being
> active, and insert the contents of the minibuffer into the echo-area
> buffer before the message text. Does anyone see problems with this?
>

I like this idea.

I think we don’t need many message-xxx like functions. Only ’message’ is enough. And it should always check if minibuffer is active and process it.

Is there cases where input prompt can be ignored and replaced?


[-- Attachment #2: Type: text/html, Size: 1259 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Wed, 11 Dec 2019 11:35:11 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: 38457@debbugs.gnu.org
> 
> Is there cases where input prompt can be ignored and replaced?

Sorry, I don't understand: what do you mean by "replaced"?

In general, prompts should not be ignored.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Tue, 10 Dec 2019 18:34:48 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 38457@debbugs.gnu.org
> 
> The original problem was, AFAIU, that various minibuffer prompts
> become obscured by echo-area display of messages.  So one possible
> solution is to modify the subroutines of 'message', e.g.,
> set_message_1, to detect the conditions of the minibuffer being
> active, and insert the contents of the minibuffer into the echo-area
> buffer before the message text.  Does anyone see problems with this?

Juri, could you please share your opinion about this proposal?  you
worked on changes related to this issues a lot lately, so you probably
know about this more than anyone else.  Do you see any problem with
the above?  If workable, I think its advantage is that it minimizes
behavior changes, while preserving the main traits of the solution: to
avoid obscuring minibuffer prompts by asynchronous messages.

TIA

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> The original problem was, AFAIU, that various minibuffer prompts
>> become obscured by echo-area display of messages.  So one possible
>> solution is to modify the subroutines of 'message', e.g.,
>> set_message_1, to detect the conditions of the minibuffer being
>> active, and insert the contents of the minibuffer into the echo-area
>> buffer before the message text.  Does anyone see problems with this?
>
> Juri, could you please share your opinion about this proposal?  you
> worked on changes related to this issues a lot lately, so you probably
> know about this more than anyone else.  Do you see any problem with
> the above?  If workable, I think its advantage is that it minimizes
> behavior changes, while preserving the main traits of the solution: to
> avoid obscuring minibuffer prompts by asynchronous messages.

The problem is not in implementation.  Of course, it's possible
to display the minibuffer's contents with an appended message
in the echo-area like Isearch does.

The problem is in usability: it would be very annoying if the message
displayed at the end of the minibuffer's contents would not vanish
after some time.  minibuffer-message removes the message
after 2 sec by default.

If someone wants the message to hang out indefinitely in the minibuffer,
this is possible, minibuffer-message-timeout is configurable:

  minibuffer-message-timeout is a variable defined in ‘C source code’.
  Its value is 2
  Documentation:
  How long to display an echo-area message when the minibuffer is active.
  If the value is a number, it should be specified in seconds.
  If the value is not a number, such messages never time out.

But this means that your proposed implementation still should use timers
to remove the echo-area with the appended message after the amount of time
specified by minibuffer-message-timeout is passed (when its value is a number).

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  38457@debbugs.gnu.org
> Date: Thu, 12 Dec 2019 01:24:30 +0200
> 
> The problem is in usability: it would be very annoying if the message
> displayed at the end of the minibuffer's contents would not vanish
> after some time.  minibuffer-message removes the message
> after 2 sec by default.

But 'message' always behaved this way, so using a timeout is change in
behavior, whereas my proposal leaves the behavior unchanged, and just
makes the prompt still visible, so it avoids confusing the user.  User
confusion was the main issue that triggered the series of changes we
are discussing, and it will be resolved by my proposal.

> If someone wants the message to hang out indefinitely in the minibuffer,
> this is possible, minibuffer-message-timeout is configurable:

That is a user option, so we cannot change it globally.  We could bind
it temporarily, but how can we know which value to use in each and
every use case, on the level where you call minibuffer-message from
inside 'message'?

> But this means that your proposed implementation still should use timers
> to remove the echo-area with the appended message after the amount of time
> specified by minibuffer-message-timeout is passed (when its value is a number).

No, my suggestion is not to remove the message automatically at all,
i.e. leave this aspect of 'message's behavior unchanged.  The message
text will be removed when either the user types something, or when
some Lisp calls 'message' again to clear the message text.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 12.12.2019 7:36, Eli Zaretskii wrote:
>> From: Juri Linkov<juri@linkov.net>
>> Cc:monnier@iro.umontreal.ca,38457@debbugs.gnu.org
>> Date: Thu, 12 Dec 2019 01:24:30 +0200
>>
>> The problem is in usability: it would be very annoying if the message
>> displayed at the end of the minibuffer's contents would not vanish
>> after some time.  minibuffer-message removes the message
>> after 2 sec by default.
> But 'message' always behaved this way, so using a timeout is change in
> behavior, whereas my proposal leaves the behavior unchanged, and just
> makes the prompt still visible, so it avoids confusing the user.  User
> confusion was the main issue that triggered the series of changes we
> are discussing, and it will be resolved by my proposal.

I think that makes a lot of sense.

Since the minibuffer is not obscured, there is no such urgency to remove 
the message on a timer, and it might even be better for usability to 
keep it. Backward compatibility also requires that.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> The problem is in usability: it would be very annoying if the message
>> displayed at the end of the minibuffer's contents would not vanish
>> after some time.  minibuffer-message removes the message
>> after 2 sec by default.
>
> But 'message' always behaved this way, so using a timeout is change in
> behavior, whereas my proposal leaves the behavior unchanged, and just
> makes the prompt still visible, so it avoids confusing the user.  User
> confusion was the main issue that triggered the series of changes we
> are discussing, and it will be resolved by my proposal.

But 'minibuffer-message' never behaved this way because it's very annoying
when messages remain indefinitely in the minibuffer, and a key is needed
to be pressed to flush mostly useless messages away.

Messages in the echo-area and messages in the minibuffer are different
things for user interaction.  Most messages are useless, but they don't
get into the way of editing in a buffer when the minibuffer is not active
and useless messages are displayed in the echo-area far away from the
editing area in the buffer.

OTOH, such messages as "Compilation finished" would significantly impact
editing of the minibuffer's content in a negative way when displayed
permanently.

>> If someone wants the message to hang out indefinitely in the minibuffer,
>> this is possible, minibuffer-message-timeout is configurable:
>
> That is a user option, so we cannot change it globally.  We could bind
> it temporarily, but how can we know which value to use in each and
> every use case, on the level where you call minibuffer-message from
> inside 'message'?

I meant that it should be possible to customize the user option.

>> But this means that your proposed implementation still should use timers
>> to remove the echo-area with the appended message after the amount of time
>> specified by minibuffer-message-timeout is passed (when its value is a number).
>
> No, my suggestion is not to remove the message automatically at all,
> i.e. leave this aspect of 'message's behavior unchanged.  The message
> text will be removed when either the user types something, or when
> some Lisp calls 'message' again to clear the message text.

It should take into account a user option that specifies the timeout
after which the message should be removed using a timer.

If you want to leave the message indefinitely by default that's fine,
but the users should have an option not to suffer from the
default behavior that you propose.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  38457@debbugs.gnu.org
> Date: Fri, 13 Dec 2019 01:07:39 +0200
> 
> > But 'message' always behaved this way, so using a timeout is change in
> > behavior, whereas my proposal leaves the behavior unchanged, and just
> > makes the prompt still visible, so it avoids confusing the user.  User
> > confusion was the main issue that triggered the series of changes we
> > are discussing, and it will be resolved by my proposal.
> 
> But 'minibuffer-message' never behaved this way because it's very annoying
> when messages remain indefinitely in the minibuffer, and a key is needed
> to be pressed to flush mostly useless messages away.
> 
> Messages in the echo-area and messages in the minibuffer are different
> things for user interaction.

That is exactly my point: these two APIs are very different in the
behavior they provide and support.  It follows that calling
'minibuffer-message' from 'message' under some circumstances is a
radical change in behavior.  Such radical changes are fine when we
introduce radical new UI features, but the changes we discuss here
were made to fix bugs.

I'm saying we can fix those bugs without introducing radical new
features, which AFAIU are not yet finished, and will probably require
many more changes to get them right.  We should limit ourselves to
fixing the most annoying bugs/misfeatures, and leave the more radical
changes for a future major release after Emacs 27.

> OTOH, such messages as "Compilation finished" would significantly impact
> editing of the minibuffer's content in a negative way when displayed
> permanently.

When the user is using the minibuffer, the message is very unlikely to
stay there indefinitely, since user interaction implies user input,
which will remove the message.  OTOH, leaving such messages until the
next input event will let the user determine when will the message be
removed, whereas using a timeout takes that control from the user.
What if the message is important, but the user takes a long time to
read it, or is distracted by something?

> >> If someone wants the message to hang out indefinitely in the minibuffer,
> >> this is possible, minibuffer-message-timeout is configurable:
> >
> > That is a user option, so we cannot change it globally.  We could bind
> > it temporarily, but how can we know which value to use in each and
> > every use case, on the level where you call minibuffer-message from
> > inside 'message'?
> 
> I meant that it should be possible to customize the user option.

And I was talking about the default operation.  The default operation
should be reasonable.

> > No, my suggestion is not to remove the message automatically at all,
> > i.e. leave this aspect of 'message's behavior unchanged.  The message
> > text will be removed when either the user types something, or when
> > some Lisp calls 'message' again to clear the message text.
> 
> It should take into account a user option that specifies the timeout
> after which the message should be removed using a timer.
> 
> If you want to leave the message indefinitely by default that's fine,
> but the users should have an option not to suffer from the
> default behavior that you propose.

We already have that default behavior.  We had it for eons.  I just
propose not to change it yet, because the alternative means a radical
change in the UI and many changes in low-level infrastructure whose
full extent we don't know yet.  The way you've decided to solve these
problems practically requires us to continue making these radical
changes, and that will delay the Emacs 27 release for many moons.

And the timing of removal of the message is just one aspect.  There
are others: the logging in *Messages*, the debug-on-message feature,
and I'm sure we will discover more of them.  The only "easy" way of
solving these is to make 'minibuffer-message' do that stuff as well,
which means incompatible changes in 'minibuffer-message', and is
likely to raise more issues, which will delay Emacs 27 even more.  All
that is the consequence of the basic fact that we both agree on:
'message' and 'minibuffer-message' are two very different beasts, so
making one call the other has got to cause complications.

If we want to release Emacs 27.1 soon (and we should), we cannot
continue on this path, we must find a reasonable way of delaying the
more radical parts of these changes to Emacs 28.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> Such variable already exists.  It's called message-in-echo-area.
>> You can enable it in the release branch if you want.
>> But then please reopen bug#34614, bug#19064, bug#17272, bug#446.
>
> Sorry, I don't understand the proposal.  How will this variable help
> if we leave the current code in 'message' as it is?  And what do you
> mean by "enabling" message-in-echo-area?

When message-in-echo-area is non-nil, then 'message' has exactly
the same behavior as before, as in the previous versions.

I propose to use timers in minibuffer-message conditionally
only when message-in-echo-area is nil.  Then the behavior
of minibuffer-message will be exactly the same as before too
when message-in-echo-area is non-nil.

If you want to set message-in-echo-area to t in the release branch,
that's fine.

Then everyone who doesn't fear timers can customize it to nil.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org,  stephen.berman@gmx.net
> Date: Thu, 12 Dec 2019 01:28:10 +0200
> 
> When message-in-echo-area is non-nil, then 'message' has exactly
> the same behavior as before, as in the previous versions.
> 
> I propose to use timers in minibuffer-message conditionally
> only when message-in-echo-area is nil.  Then the behavior
> of minibuffer-message will be exactly the same as before too
> when message-in-echo-area is non-nil.
> 
> If you want to set message-in-echo-area to t in the release branch,
> that's fine.

That would mean bring back all the problems for which these changes
were made.  So it would be the worst of all worlds, and thus makes
very little sense to me.

And in any case, minibuffer-message already behaves differently: it
logs the message in *Messages*, something it never did and is not
documented, and you suggested another change, to make it start the
debugger per debug-on-message.  These change behavior of any direct
callers of minibuffer-message in incompatible ways, something I don't
think we have a good reason to do.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> When message-in-echo-area is non-nil, then 'message' has exactly
>> the same behavior as before, as in the previous versions.
>>
>> I propose to use timers in minibuffer-message conditionally
>> only when message-in-echo-area is nil.  Then the behavior
>> of minibuffer-message will be exactly the same as before too
>> when message-in-echo-area is non-nil.
>>
>> If you want to set message-in-echo-area to t in the release branch,
>> that's fine.
>
> That would mean bring back all the problems for which these changes
> were made.  So it would be the worst of all worlds, and thus makes
> very little sense to me.

Then your proposed implementation should be activated when
minibuffer-message-timeout is set to a non-nil value.
Otherwise, when it's a number, it should use the timer.

> And in any case, minibuffer-message already behaves differently: it
> logs the message in *Messages*, something it never did and is not
> documented, and you suggested another change, to make it start the
> debugger per debug-on-message.  These change behavior of any direct
> callers of minibuffer-message in incompatible ways, something I don't
> think we have a good reason to do.

I see no reason not to change minibuffer-message.  But if you think
it should never change, let's create a duplicate function
message-in-minibuffer.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Fri, 13 Dec 2019 01:12:50 +0200
> 
> >> If you want to set message-in-echo-area to t in the release branch,
> >> that's fine.
> >
> > That would mean bring back all the problems for which these changes
> > were made.  So it would be the worst of all worlds, and thus makes
> > very little sense to me.
> 
> Then your proposed implementation should be activated when
> minibuffer-message-timeout is set to a non-nil value.
> Otherwise, when it's a number, it should use the timer.

That leaves open the issue of the default value of
minibuffer-message-timeout.  I don't think we can change it, because
it affects minibuffer-message as well.  But we could have a new
option, which would affect only the duplicate function you mention
below.  If the new option by default makes the message stay until the
next one or until user input, I think this would be an okay solution
that satisfies everyone, at least for Emacs 27.

> > And in any case, minibuffer-message already behaves differently: it
> > logs the message in *Messages*, something it never did and is not
> > documented, and you suggested another change, to make it start the
> > debugger per debug-on-message.  These change behavior of any direct
> > callers of minibuffer-message in incompatible ways, something I don't
> > think we have a good reason to do.
> 
> I see no reason not to change minibuffer-message.  But if you think
> it should never change, let's create a duplicate function
> message-in-minibuffer.

Fine with me, as long as the duplicate is an internal function.  Maybe
that new internal function should be invoked from message3_nolog
instead, btw?  That would remove the need to duplicate the
functionality of message_dolog.

Assuming you agree, once this change is made, some of the recent
changes related to these issues should be reverted.  Can I ask you to
review those related changesets and publish a list of those which need
to be reverted or augmented?

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> > That would mean bring back all the problems for which these changes
>> > were made.  So it would be the worst of all worlds, and thus makes
>> > very little sense to me.
>> 
>> Then your proposed implementation should be activated when
>> minibuffer-message-timeout is set to a non-nil value.
>> Otherwise, when it's a number, it should use the timer.
>
> That leaves open the issue of the default value of
> minibuffer-message-timeout.  I don't think we can change it, because
> it affects minibuffer-message as well.  But we could have a new
> option, which would affect only the duplicate function you mention
> below.  If the new option by default makes the message stay until the
> next one or until user input, I think this would be an okay solution
> that satisfies everyone, at least for Emacs 27.

The main problem with 'minibuffer-message' is that it uses 'sit-for'.
If a new function will use the same 'sit-for' to detect when input is
available, then it will bring the same problems.  Here's is an example
of the problems it causes: I call 'select-window' from 'post-command-hook'
to select an output window, but 'sit-for' delays selecting the window
for 2 seconds until 'execute-extended-command' finishes displaying
the message "You can run the command `%s' with %s" using 'sit-for'.
The output window becomes selected only after 2 seconds because
a customized function in 'post-command-hook' doesn't run earlier
than returning from 'sit-for' in 'execute-extended-command'.

'sit-for' is used in many places, but it's inappropriate for these tasks.
What is a possible replacement for 'sit-for'?  For a delay of specified
seconds, one solution is to run a timer.  But how to replace another
aspect of 'sit-for', namely an ability to stop when input is available?

There is a need for a hook that would be called when input is available.
Maybe such hook already exists, I don't know, I searched but found only
'echo-area-clear-hook' that is used in this code in 'command_loop_1':

      if (minibuf_level
	  && !NILP (echo_area_buffer[0])
	  && EQ (minibuf_window, echo_area_window)
	  && NUMBERP (Vminibuffer_message_timeout))
	{
	  /* Bind inhibit-quit to t so that C-g gets read in
	     rather than quitting back to the minibuffer.  */
	  ptrdiff_t count = SPECPDL_INDEX ();
	  specbind (Qinhibit_quit, Qt);

	  sit_for (Vminibuffer_message_timeout, 0, 2);

	  /* Clear the echo area.  */
	  message1 (0);
	  safe_run_hooks (Qecho_area_clear_hook);

	  unbind_to (count, Qnil);

	  /* If a C-g came in before, treat it as input now.  */
	  if (!NILP (Vquit_flag))
	    {
	      Vquit_flag = Qnil;
	      Vunread_command_events = list1i (quit_char);
	    }
	}

This code looks very like 'minibuffer-message', and still uses 'sit_for'.

>> > And in any case, minibuffer-message already behaves differently: it
>> > logs the message in *Messages*, something it never did and is not
>> > documented, and you suggested another change, to make it start the
>> > debugger per debug-on-message.  These change behavior of any direct
>> > callers of minibuffer-message in incompatible ways, something I don't
>> > think we have a good reason to do.
>> 
>> I see no reason not to change minibuffer-message.  But if you think
>> it should never change, let's create a duplicate function
>> message-in-minibuffer.
>
> Fine with me, as long as the duplicate is an internal function.  Maybe
> that new internal function should be invoked from message3_nolog
> instead, btw?  That would remove the need to duplicate the
> functionality of message_dolog.
>
> Assuming you agree, once this change is made, some of the recent
> changes related to these issues should be reverted.  Can I ask you to
> review those related changesets and publish a list of those which need
> to be reverted or augmented?

Here's is a list of related changesets:

8693611136
aa89c84e00
54c792ece6

Please revert them if you want.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Sun, 15 Dec 2019 01:10:58 +0200
> 
> > That leaves open the issue of the default value of
> > minibuffer-message-timeout.  I don't think we can change it, because
> > it affects minibuffer-message as well.  But we could have a new
> > option, which would affect only the duplicate function you mention
> > below.  If the new option by default makes the message stay until the
> > next one or until user input, I think this would be an okay solution
> > that satisfies everyone, at least for Emacs 27.
> 
> The main problem with 'minibuffer-message' is that it uses 'sit-for'.
> If a new function will use the same 'sit-for' to detect when input is
> available, then it will bring the same problems.  Here's is an example
> of the problems it causes: I call 'select-window' from 'post-command-hook'
> to select an output window, but 'sit-for' delays selecting the window
> for 2 seconds until 'execute-extended-command' finishes displaying
> the message "You can run the command `%s' with %s" using 'sit-for'.
> The output window becomes selected only after 2 seconds because
> a customized function in 'post-command-hook' doesn't run earlier
> than returning from 'sit-for' in 'execute-extended-command'.
> 
> 'sit-for' is used in many places, but it's inappropriate for these tasks.
> What is a possible replacement for 'sit-for'?  For a delay of specified
> seconds, one solution is to run a timer.  But how to replace another
> aspect of 'sit-for', namely an ability to stop when input is available?

What I had in mind was to go with your idea, viz.:

> Then your proposed implementation should be activated when
> minibuffer-message-timeout is set to a non-nil value.
> Otherwise, when it's a number, it should use the timer.

IOW, introduce a new option, which will affect the new function we
were talking about, a near-clone of minibuffer-message (and will not
affect minibuffer-message itself).  When that new option's value is
not a number, the near-clone of minibuffer-message should not call
sit-for at all; and when that value is a number, use a timer to remove
the message after that many seconds if no input arrives before that.

In any case, I thought we agreed not to call minibuffer-message from
'message', but define a new function, similar but not identical to
minibuffer-message (what I call a "near-clone").  And
minibuffer-message-timeout should not affect that new function, it
should be a separate option.

> There is a need for a hook that would be called when input is available.
> Maybe such hook already exists, I don't know, I searched but found only
> 'echo-area-clear-hook' that is used in this code in 'command_loop_1':

Do we still need such a hook if the idea above is implemented instead?

> > Assuming you agree, once this change is made, some of the recent
> > changes related to these issues should be reverted.  Can I ask you to
> > review those related changesets and publish a list of those which need
> > to be reverted or augmented?
> 
> Here's is a list of related changesets:
> 
> 8693611136
> aa89c84e00
> 54c792ece6
> 
> Please revert them if you want.

Thanks.  Let's revisit these after the implementation of the
minibuffer-message's clone is finalized, so that we could know which
ones of these are still needed and which aren't.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

>> The main problem with 'minibuffer-message' is that it uses 'sit-for'.
>> If a new function will use the same 'sit-for' to detect when input is
>> available, then it will bring the same problems.  Here's is an example
>> of the problems it causes: I call 'select-window' from 'post-command-hook'
>> to select an output window, but 'sit-for' delays selecting the window
>> for 2 seconds until 'execute-extended-command' finishes displaying
>> the message "You can run the command `%s' with %s" using 'sit-for'.
>> The output window becomes selected only after 2 seconds because
>> a customized function in 'post-command-hook' doesn't run earlier
>> than returning from 'sit-for' in 'execute-extended-command'.
>>
>> 'sit-for' is used in many places, but it's inappropriate for these tasks.
>> What is a possible replacement for 'sit-for'?  For a delay of specified
>> seconds, one solution is to run a timer.  But how to replace another
>> aspect of 'sit-for', namely an ability to stop when input is available?

I'd like to add that the biggest drawback of sit-for is that
no message is displayed when input is available like is in e.g.
do-after-load-evaluation after start, or when the user is typing in
the minibuffer, the user might miss an important message.

> What I had in mind was to go with your idea, viz.:
>
>> Then your proposed implementation should be activated when
>> minibuffer-message-timeout is set to a non-nil value.
>> Otherwise, when it's a number, it should use the timer.
>
> IOW, introduce a new option, which will affect the new function we
> were talking about, a near-clone of minibuffer-message (and will not
> affect minibuffer-message itself).  When that new option's value is
> not a number, the near-clone of minibuffer-message should not call
> sit-for at all; and when that value is a number, use a timer to remove
> the message after that many seconds if no input arrives before that.
>
> In any case, I thought we agreed not to call minibuffer-message from
> 'message', but define a new function, similar but not identical to
> minibuffer-message (what I call a "near-clone").  And
> minibuffer-message-timeout should not affect that new function, it
> should be a separate option.

I agree that a new function needs to be created, and agree
not to call minibuffer-message from 'message'.

>> > Assuming you agree, once this change is made, some of the recent
>> > changes related to these issues should be reverted.  Can I ask you to
>> > review those related changesets and publish a list of those which need
>> > to be reverted or augmented?
>>
>> Here's is a list of related changesets:
>>
>> 8693611136
>> aa89c84e00
>> 54c792ece6
>>
>> Please revert them if you want.
>
> Thanks.  Let's revisit these after the implementation of the
> minibuffer-message's clone is finalized, so that we could know which
> ones of these are still needed and which aren't.

Since a new near-clone of minibuffer-message is needed, and
we agreed to not call minibuffer-message from 'message',
I reverted all previous changes.

Now a new feature could be implemented from scratch.

I created a new ELPA package attached below that
implements this feature.  So everyone who like it
can use it even after release.  Also this package
could be used for experimentation to find the best way
to use it, that later could be moved to core in Emacs 28.

I think now we have no moral right to delay the Emacs 27 pretest
anymore.


[-- Attachment #2: minimess.el --]
[-- Type: application/emacs-lisp, Size: 5263 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Mon, 16 Dec 2019 01:59:45 +0200
> 
> > IOW, introduce a new option, which will affect the new function we
> > were talking about, a near-clone of minibuffer-message (and will not
> > affect minibuffer-message itself).  When that new option's value is
> > not a number, the near-clone of minibuffer-message should not call
> > sit-for at all; and when that value is a number, use a timer to remove
> > the message after that many seconds if no input arrives before that.
> >
> > In any case, I thought we agreed not to call minibuffer-message from
> > 'message', but define a new function, similar but not identical to
> > minibuffer-message (what I call a "near-clone").  And
> > minibuffer-message-timeout should not affect that new function, it
> > should be a separate option.
> 
> I agree that a new function needs to be created, and agree
> not to call minibuffer-message from 'message'.

OK.

> >> 8693611136
> >> aa89c84e00
> >> 54c792ece6
> >>
> >> Please revert them if you want.
> >
> > Thanks.  Let's revisit these after the implementation of the
> > minibuffer-message's clone is finalized, so that we could know which
> > ones of these are still needed and which aren't.
> 
> Since a new near-clone of minibuffer-message is needed, and
> we agreed to not call minibuffer-message from 'message',
> I reverted all previous changes.
> 
> Now a new feature could be implemented from scratch.
> 
> I created a new ELPA package attached below that
> implements this feature.  So everyone who like it
> can use it even after release.  Also this package
> could be used for experimentation to find the best way
> to use it, that later could be moved to core in Emacs 28.
> 
> I think now we have no moral right to delay the Emacs 27 pretest
> anymore.

Thanks.

However, I hoped we could have at least part of what you did in Emacs
27, and not lose all of it.  We could install a function like
'minimess-minibuffer-message', and make one of the subroutines of
'message' call it under the right conditions.  Then the message which
'minimess-minibuffer-message' displays could be removed by a timer if
the new timeout defcustom is a number, and if that defcustom is not a
number (which will be the default), the message stays and sit-for is
not called.

Do you see any danger with the above?  If not, can I persuade you do
install such a change?

> (defun minimess-message (orig-fun format-string &rest args)
>   (if (and
>        ;; When `inhibit-message' is non-nil, the intention was to just
>        ;; log the message to the *Messages* buffer using `message'.
>        (null inhibit-message)
>        (window-live-p (active-minibuffer-window))
>        (window-live-p (old-selected-window))
>        (bufferp (window-buffer (old-selected-window)))
>        (minibufferp (window-buffer (old-selected-window))))

Btw, can you explain why every part of this condition is needed?  IOW,
why isn't just the below enough?

   (window-live-p (active-minibuffer-window))

(I do understand the reason for the test of inhibit-message).

Maybe the other conditions need a comment to explain them?

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> However, I hoped we could have at least part of what you did in Emacs
> 27, and not lose all of it.  We could install a function like
> 'minimess-minibuffer-message', and make one of the subroutines of
> 'message' call it under the right conditions.

The most low-level functions where it could be called from
are 'set_message' and 'clear_message'.

> Then the message which
> 'minimess-minibuffer-message' displays could be removed by a timer if
> the new timeout defcustom is a number, and if that defcustom is not a
> number (which will be the default), the message stays and sit-for is
> not called.

This is almost how 'minimess-minibuffer-message' was implemented,
with one difference: defcustom is a number by default.
Why shouldn't it be a number by default?  How the user would be able
to remove an old message when the function doesn't call sit-for?

> Do you see any danger with the above?  If not, can I persuade you do
> install such a change?

I'm not sure if now is the right time to implement this feature.
But if implementation would be straightforward and if you see no problems
then why not.

>> (defun minimess-message (orig-fun format-string &rest args)
>>   (if (and
>>        ;; When `inhibit-message' is non-nil, the intention was to just
>>        ;; log the message to the *Messages* buffer using `message'.
>>        (null inhibit-message)
>>        (window-live-p (active-minibuffer-window))
>>        (window-live-p (old-selected-window))
>>        (bufferp (window-buffer (old-selected-window)))
>>        (minibufferp (window-buffer (old-selected-window))))
>
> Btw, can you explain why every part of this condition is needed?  IOW,
> why isn't just the below enough?
>
>    (window-live-p (active-minibuffer-window))
>
> (I do understand the reason for the test of inhibit-message).
>
> Maybe the other conditions need a comment to explain them?

Unfortunately, I forgot why they were added, i.e. during testing
I added them one by one when noticed that some cases don't work.
Now I'll try to reproduce these cases by removing conditions
and checking which part doesn't work.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 17.12.2019 0:29, Juri Linkov wrote:
> How the user would be able
> to remove an old message when the function doesn't call sit-for?

Just like they did for decades until now: by pressing (any) button. 
Which will invoke the next command and clear the echo area.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 231 bytes --]

For the original bug of this thread, I have a feature request:
I would like dabbrev-expand to don’t display the messages when minibuffer is active. It is annoying in everyday use. It cause flickers. I suffer from it too long.

[-- Attachment #2: Type: text/html, Size: 543 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Date: Tue, 17 Dec 2019 14:27:38 +0800
> From: HaiJun Zhang <netjune@outlook.com>
> Cc: 38457@debbugs.gnu.org
> 
> For the original bug of this thread, I have a feature request: 
> I would like dabbrev-expand to don’t display the messages when minibuffer is active. It is annoying in everyday
> use. It cause flickers. I suffer from it too long.

Is that wise?  dabbrev-expand could take quite some time, if it
starts looking in other buffers and there are a lot of them.  If it
keeps silence, you will see Emacs frozen without any indication of
what's going on.  People will consider this a bug and complain.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> For the original bug of this thread, I have a feature request:
> I would like dabbrev-expand to don’t display the messages when
> minibuffer is active.  It is annoying in everyday use.  It cause
> flickers.  I suffer from it too long.

Don't worry.  With the new patch you will see no messages from dabbrev-expand
because at the end dabbrev-expand already calls '(message nil)' that
quickly clears the message from the minibuffer.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[HaiJun Zhang]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

在 2019年12月18日 +0800 AM8:06，Juri Linkov <juri@linkov.net>，写道：
> > For the original bug of this thread, I have a feature request:
> > I would like dabbrev-expand to don’t display the messages when
> > minibuffer is active. It is annoying in everyday use. It cause
> > flickers. I suffer from it too long.
>
> Don't worry. With the new patch you will see no messages from dabbrev-expand
> because at the end dabbrev-expand already calls '(message nil)' that
> quickly clears the message from the minibuffer.

Yes. I’m waiting for it.
Thanks for your great work.

[-- Attachment #2: Type: text/html, Size: 1141 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Tue, 17 Dec 2019 00:29:32 +0200
> 
> > However, I hoped we could have at least part of what you did in Emacs
> > 27, and not lose all of it.  We could install a function like
> > 'minimess-minibuffer-message', and make one of the subroutines of
> > 'message' call it under the right conditions.
> 
> The most low-level functions where it could be called from
> are 'set_message' and 'clear_message'.

I had in mind set_message_1 rather than set_message, but the
difference is not critical for this discussion.

> > Then the message which
> > 'minimess-minibuffer-message' displays could be removed by a timer if
> > the new timeout defcustom is a number, and if that defcustom is not a
> > number (which will be the default), the message stays and sit-for is
> > not called.
> 
> This is almost how 'minimess-minibuffer-message' was implemented,
> with one difference: defcustom is a number by default.
> Why shouldn't it be a number by default?

By making the default not a number, we keep the previous behavior of
'message' in this aspect, thus minimizing potential unintended
consequences.  At the same time, we gain another aspect: we avoid
hiding the minibuffer prompt.  So this will be a partial improvement
wrt the current behavior, with an option for users who would like that
to customize the value to a number, and thus get the message removed
automatically after some delay.

> How the user would be able to remove an old message when the
> function doesn't call sit-for?

Like they do today: type something.  And in some situations, not even
that, if the Lisp program which displayed the message will shortly
follow up with clearing the echo area (that's what happens with
dabbrev, for example).

> > Do you see any danger with the above?  If not, can I persuade you do
> > install such a change?
> 
> I'm not sure if now is the right time to implement this feature.
> But if implementation would be straightforward and if you see no problems
> then why not.

The implementation looks straightforward to me, since you already
implemented almost all of it in that ELPA package.  What's left is
minor details.

> >>   (if (and
> >>        ;; When `inhibit-message' is non-nil, the intention was to just
> >>        ;; log the message to the *Messages* buffer using `message'.
> >>        (null inhibit-message)
> >>        (window-live-p (active-minibuffer-window))
> >>        (window-live-p (old-selected-window))
> >>        (bufferp (window-buffer (old-selected-window)))
> >>        (minibufferp (window-buffer (old-selected-window))))
> >
> > Btw, can you explain why every part of this condition is needed?  IOW,
> > why isn't just the below enough?
> >
> >    (window-live-p (active-minibuffer-window))
> >
> > (I do understand the reason for the test of inhibit-message).
> >
> > Maybe the other conditions need a comment to explain them?
> 
> Unfortunately, I forgot why they were added, i.e. during testing
> I added them one by one when noticed that some cases don't work.
> Now I'll try to reproduce these cases by removing conditions
> and checking which part doesn't work.

Thank you.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

>> >>   (if (and
>> >>        ;; When `inhibit-message' is non-nil, the intention was to just
>> >>        ;; log the message to the *Messages* buffer using `message'.
>> >>        (null inhibit-message)
>> >>        (window-live-p (active-minibuffer-window))
>> >>        (window-live-p (old-selected-window))
>> >>        (bufferp (window-buffer (old-selected-window)))
>> >>        (minibufferp (window-buffer (old-selected-window))))
>> >
>> > Btw, can you explain why every part of this condition is needed?  IOW,
>> > why isn't just the below enough?
>> >
>> >    (window-live-p (active-minibuffer-window))
>> >
>> > (I do understand the reason for the test of inhibit-message).
>> >
>> > Maybe the other conditions need a comment to explain them?
>> 
>> Unfortunately, I forgot why they were added, i.e. during testing
>> I added them one by one when noticed that some cases don't work.
>> Now I'll try to reproduce these cases by removing conditions
>> and checking which part doesn't work.

Now I recall why just (window-live-p (active-minibuffer-window))
is not enough.  Because it returns non-nil even when the
current buffer is not the minibuffer, but the minibuffer was
activated earlier.  Test case:

0. emacs -Q
1. M-x   ;; activate the minibuffer
2. C-x o ;; switch back to *scratch*
3. Eval in *scratch* buffer:

   (window-live-p (active-minibuffer-window))
   => t

A message overlay should not be added to the *scratch* buffer, so it's
important to check if old-selected-window is a minibuffer window
(i.e. the current buffer is the minibuffer).

>> But if implementation would be straightforward and if you see no problems
>> then why not.
>
> The implementation looks straightforward to me, since you already
> implemented almost all of it in that ELPA package.  What's left is
> minor details.

Let's iron out the details.  A new patch attached works well
in all cases I tested (dabbrev, icomplete, etc.)  But I'm sure
it could be improved further because I might have made wrong
assumptions on the C side, or something.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: set-minibuffer-message.patch --]
[-- Type: text/x-diff, Size: 6366 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 76d8ca4475..331b2a44ed 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -746,6 +746,78 @@ minibuffer-message
             (sit-for (or minibuffer-message-timeout 1000000)))
         (delete-overlay ol)))))
 
+(defcustom minibuffer-message-wait nil
+  "How long to display an echo-area message when the minibuffer is active.
+If the value is a number, it should be specified in seconds.
+If the value is not a number, such messages never time out."
+  :type '(choice (const :tag "Never time out" nil)
+                 (integer :tag "Wait for the number of seconds" 2)))
+
+(defvar minibuffer-message-timer nil)
+(defvar minibuffer-message-overlay nil)
+
+(defun set-minibuffer-message (message)
+  "Temporarily display MESSAGE at the end of the minibuffer.
+The text is displayed for `minibuffer-message-wait' seconds,
+or until the next input event arrives, whichever comes first.
+Enclose MESSAGE in [...] if this is not yet the case."
+  (when (and (not noninteractive)
+             (window-live-p (active-minibuffer-window))
+             (window-live-p (old-selected-window))
+             (bufferp (window-buffer (old-selected-window)))
+             (minibufferp (window-buffer (old-selected-window))))
+    (setq message (if (string-match-p "\\` *\\[.+\\]\\'" message)
+                      ;; Make sure we can put-text-property.
+                      (copy-sequence message)
+                    (concat " [" message "]")))
+    (unless (or (null minibuffer-message-properties)
+                ;; Don't overwrite the face properties the caller has set
+                (text-properties-at 0 message))
+      (setq message (apply #'propertize message minibuffer-message-properties)))
+
+    (when (timerp minibuffer-message-timer)
+      (cancel-timer minibuffer-message-timer)
+      (setq minibuffer-message-timer nil))
+    (when (overlayp minibuffer-message-overlay)
+      (delete-overlay minibuffer-message-overlay)
+      (setq minibuffer-message-overlay nil))
+
+    (setq minibuffer-message-overlay
+          (make-overlay (point-max) (point-max) nil t t))
+    (unless (zerop (length 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))
+    (overlay-put minibuffer-message-overlay 'after-string message)
+
+    (when (numberp minibuffer-message-wait)
+      (setq minibuffer-message-timer
+            (run-with-timer minibuffer-message-wait nil
+                            (lambda ()
+                              (when (overlayp minibuffer-message-overlay)
+                                (delete-overlay minibuffer-message-overlay)
+                                (setq minibuffer-message-overlay nil))))))
+
+    t))
+
+(setq set-message-function 'set-minibuffer-message)
+
+(defun clear-minibuffer-message ()
+  "Clear minibuffer message."
+  (when (not noninteractive)
+    ;; When this option is a number, the message
+    ;; should be cleared only by timer.
+    (unless (numberp minibuffer-message-wait)
+      (when (timerp minibuffer-message-timer)
+        (cancel-timer minibuffer-message-timer)
+        (setq minibuffer-message-timer nil))
+      (when (overlayp minibuffer-message-overlay)
+        (delete-overlay minibuffer-message-overlay)
+        (setq minibuffer-message-overlay nil)))))
+
+(setq clear-message-function 'clear-minibuffer-message)
+
 (defun minibuffer-completion-contents ()
   "Return the user input in a minibuffer before point as a string.
 In Emacs 22, that was what completion commands operated on.
diff --git a/src/keyboard.c b/src/keyboard.c
index 5135fd0bc8..d9c9213098 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2990,6 +2990,8 @@ read_char (int commandflag, Lisp_Object map,
 	  safe_run_hooks (Qecho_area_clear_hook);
 	  clear_message (1, 0);
 	}
+      else if (!NILP (Vclear_message_function))
+        message1 (0);
     }
 
  reread_for_input_method:
diff --git a/src/xdisp.c b/src/xdisp.c
index 08c6927052..a31a90c7e2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11706,13 +11706,33 @@ truncate_message_1 (ptrdiff_t nchars, Lisp_Object a2)
 static void
 set_message (Lisp_Object string)
 {
+  Lisp_Object message = Qnil;
+
   eassert (STRINGP (string));
 
-  message_enable_multibyte = STRING_MULTIBYTE (string);
+  if (!NILP (Vset_message_function))
+    {
+      ptrdiff_t count = SPECPDL_INDEX ();
+      specbind (Qinhibit_quit, Qt);
+      message = call1 (Vset_message_function, string);
+      unbind_to (count, Qnil);
 
-  with_echo_area_buffer (0, -1, set_message_1, 0, string);
-  message_buf_print = false;
-  help_echo_showing_p = false;
+      if (STRINGP (message))
+        {
+          eassert (STRINGP (message));
+          string = message;
+          message = Qnil;
+        }
+    }
+
+  if (NILP (message))
+    {
+      message_enable_multibyte = STRING_MULTIBYTE (string);
+
+      with_echo_area_buffer (0, -1, set_message_1, 0, string);
+      message_buf_print = false;
+      help_echo_showing_p = false;
+    }
 
   if (STRINGP (Vdebug_on_message)
       && STRINGP (string)
@@ -11768,6 +11788,14 @@ clear_message (bool current_p, bool last_displayed_p)
     {
       echo_area_buffer[0] = Qnil;
       message_cleared_p = true;
+
+      if (!NILP (Vclear_message_function))
+        {
+          ptrdiff_t count = SPECPDL_INDEX ();
+          specbind (Qinhibit_quit, Qt);
+          call0 (Vclear_message_function);
+          unbind_to (count, Qnil);
+        }
     }
 
   if (last_displayed_p)
@@ -34940,6 +34968,14 @@ syms_of_xdisp (void)
 	       doc: /* If non-nil, debug if a message matching this regexp is displayed.  */);
   Vdebug_on_message = Qnil;
 
+  DEFVAR_LISP ("set-message-function", Vset_message_function,
+	       doc: /* If non-nil, function to set message.  */);
+  Vset_message_function = Qnil;
+
+  DEFVAR_LISP ("clear-message-function", Vclear_message_function,
+	       doc: /* If non-nil, function to clear message.  */);
+  Vclear_message_function = Qnil;
+
   DEFVAR_LISP ("redisplay--all-windows-cause", Vredisplay__all_windows_cause,
 	       doc: /*  */);
   Vredisplay__all_windows_cause = Fmake_hash_table (0, NULL);

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Wed, 18 Dec 2019 01:51:00 +0200
> 
> Now I recall why just (window-live-p (active-minibuffer-window))
> is not enough.  Because it returns non-nil even when the
> current buffer is not the minibuffer, but the minibuffer was
> activated earlier.  Test case:
> 
> 0. emacs -Q
> 1. M-x   ;; activate the minibuffer
> 2. C-x o ;; switch back to *scratch*
> 3. Eval in *scratch* buffer:
> 
>    (window-live-p (active-minibuffer-window))
>    => t

OK, but the minibuffer is still active in this case, and leaving it
unobscured is still an advantage, right?

> A message overlay should not be added to the *scratch* buffer, so it's
> important to check if old-selected-window is a minibuffer window
> (i.e. the current buffer is the minibuffer).

OK, but couldn't we instead do something like

  (with-current-buffer (window-buffer (active-minibuffer-window))
    ....

to ensure we add the overlay in the minibuffer, not in *scratch*?  Or
am I missing something?

> Let's iron out the details.  A new patch attached works well
> in all cases I tested (dabbrev, icomplete, etc.)  But I'm sure
> it could be improved further because I might have made wrong
> assumptions on the C side, or something.

Thanks, I have only a few minor comments:

> +(defun set-minibuffer-message (message)
> +  "Temporarily display MESSAGE at the end of the minibuffer.
> +The text is displayed for `minibuffer-message-wait' seconds,
> +or until the next input event arrives, whichever comes first.

This text needs to be updated to refer to minibuffer-message-wait's
effect on what it does.

> +      else if (!NILP (Vclear_message_function))
> +        message1 (0);

Here and elsewhere, isn't it better to use FUNCTIONP instead of NILP?

> +      if (STRINGP (message))
> +        {
> +          eassert (STRINGP (message));

Since you just verified that 'message' is a string, the eassert is
redundant, right?

> +      message = call1 (Vset_message_function, string);

I'd prefer to use safe_call1 here, in case the function signals an
error, since we are inside redisplay here...

> +          call0 (Vclear_message_function);

...and safe_call here.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> Now I recall why just (window-live-p (active-minibuffer-window))
>> is not enough.  Because it returns non-nil even when the
>> current buffer is not the minibuffer, but the minibuffer was
>> activated earlier.  Test case:
>>
>> 0. emacs -Q
>> 1. M-x   ;; activate the minibuffer
>> 2. C-x o ;; switch back to *scratch*
>> 3. Eval in *scratch* buffer:
>>
>>    (window-live-p (active-minibuffer-window))
>>    => t
>
> OK, but the minibuffer is still active in this case, and leaving it
> unobscured is still an advantage, right?

It never occurred to me that someone might want to see the minibuffer
unobscured when the minibuffer is not the current buffer.

>> A message overlay should not be added to the *scratch* buffer, so it's
>> important to check if old-selected-window is a minibuffer window
>> (i.e. the current buffer is the minibuffer).
>
> OK, but couldn't we instead do something like
>
>   (with-current-buffer (window-buffer (active-minibuffer-window))
>     ....
>
> to ensure we add the overlay in the minibuffer, not in *scratch*?  Or
> am I missing something?

Maybe.  Actually I have no opinion for this case because it never was
a problem when the non-current minibuffer was obscured by messages.

>> Let's iron out the details.  A new patch attached works well
>> in all cases I tested (dabbrev, icomplete, etc.)  But I'm sure
>> it could be improved further because I might have made wrong
>> assumptions on the C side, or something.
>
> Thanks, I have only a few minor comments:
>
>> +(defun set-minibuffer-message (message)
>> +  "Temporarily display MESSAGE at the end of the minibuffer.
>> +The text is displayed for `minibuffer-message-wait' seconds,
>> +or until the next input event arrives, whichever comes first.
>
> This text needs to be updated to refer to minibuffer-message-wait's
> effect on what it does.

While thinking again about 'minibuffer-message-wait' I realized that
maybe we need two customizable variables:

1. minibuffer-message-clear-after-input
2. minibuffer-message-clear-after-timeout

because users might prefer customization of these behaviors separately.
Here are all possible combinations (2 is an example of number of seconds,
0 means no timeout):

after-input=nil after-timeout=0 - never clear the message
after-input=t   after-timeout=2 - clear either on input or after timeout
after-input=t   after-timeout=0 - clear only when input is available:
                                  this has an advantage that user has control when
                                  wants to clear message immediately on keypress;
after-input=nil after-timeout=2 - clear only after timeout, not on input:
                                  this has an advantage that user will never miss
                                  a message while typing in the minibuffer,
                                  the message will stay for the specified number
                                  of seconds regardless of input,
                                  so user will have a chance to read it

Do all these variants make sense?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Thu, 19 Dec 2019 02:12:09 +0200
> 
> >> 0. emacs -Q
> >> 1. M-x   ;; activate the minibuffer
> >> 2. C-x o ;; switch back to *scratch*
> >> 3. Eval in *scratch* buffer:
> >>
> >>    (window-live-p (active-minibuffer-window))
> >>    => t
> >
> > OK, but the minibuffer is still active in this case, and leaving it
> > unobscured is still an advantage, right?
> 
> It never occurred to me that someone might want to see the minibuffer
> unobscured when the minibuffer is not the current buffer.

I think leaving "M-x" (or any other prompt) unobscured in this
situation is a nice benefit, and if it simplifies the code, it's even
more desirable.

> >> +(defun set-minibuffer-message (message)
> >> +  "Temporarily display MESSAGE at the end of the minibuffer.
> >> +The text is displayed for `minibuffer-message-wait' seconds,
> >> +or until the next input event arrives, whichever comes first.
> >
> > This text needs to be updated to refer to minibuffer-message-wait's
> > effect on what it does.
> 
> While thinking again about 'minibuffer-message-wait' I realized that
> maybe we need two customizable variables:
> 
> 1. minibuffer-message-clear-after-input
> 2. minibuffer-message-clear-after-timeout
> 
> because users might prefer customization of these behaviors separately.
> Here are all possible combinations (2 is an example of number of seconds,
> 0 means no timeout):
> 
> after-input=nil after-timeout=0 - never clear the message
> after-input=t   after-timeout=2 - clear either on input or after timeout
> after-input=t   after-timeout=0 - clear only when input is available:
>                                   this has an advantage that user has control when
>                                   wants to clear message immediately on keypress;
> after-input=nil after-timeout=2 - clear only after timeout, not on input:
>                                   this has an advantage that user will never miss
>                                   a message while typing in the minibuffer,
>                                   the message will stay for the specified number
>                                   of seconds regardless of input,
>                                   so user will have a chance to read it
> 
> Do all these variants make sense?

I would never want to use variants 1 and 4, but if someone wants them,
I won't object as long as the default for Emacs 27 is as in the 2nd
variant.

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]

> I think leaving "M-x" (or any other prompt) unobscured in this
> situation is a nice benefit, and if it simplifies the code, it's even
> more desirable.

Implemented in a new patch.

>> after-input=nil after-timeout=0 - never clear the message
>> after-input=t   after-timeout=2 - clear either on input or after timeout
>> after-input=t   after-timeout=0 - clear only when input is available:
>>                                   this has an advantage that user has control when
>>                                   wants to clear message immediately on keypress;
>> after-input=nil after-timeout=2 - clear only after timeout, not on input:
>>                                   this has an advantage that user will never miss
>>                                   a message while typing in the minibuffer,
>>                                   the message will stay for the specified number
>>                                   of seconds regardless of input,
>>                                   so user will have a chance to read it
>>
>> Do all these variants make sense?
>
> I would never want to use variants 1 and 4, but if someone wants them,
> I won't object as long as the default for Emacs 27 is as in the 2nd
> variant.

Actually, supporting an option after-input is not straightforward,
because when clear-minibuffer-message is called, there is no distinction
whether it was called by new input arrived, or by a function calling
'message' with an empty argument.  But really an option after-input
is not necessary, this rules out variants 1 and 4 anyway.

So this patch implements only the option clear-timeout
with the 2nd variant by default.

Also made changes for all your previous comments:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: set-minibuffer-message-2.patch --]
[-- Type: text/x-diff, Size: 6309 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 76d8ca4475..502375ee1e 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -746,6 +746,76 @@ minibuffer-message
             (sit-for (or minibuffer-message-timeout 1000000)))
         (delete-overlay ol)))))
 
+(defcustom minibuffer-message-clear-timeout 2
+  "How long to display an echo-area message when the minibuffer is active.
+If the value is a number, it should be specified in seconds.
+If the value is not a number, such messages never time out,
+and the text is displayed until the next input event arrives."
+  :type '(choice (const :tag "Never time out" nil)
+                 (integer :tag "Wait for the number of seconds" 2))
+  :version "27.1")
+
+(defvar minibuffer-message-timer nil)
+(defvar minibuffer-message-overlay nil)
+
+(defun set-minibuffer-message (message)
+  "Temporarily display MESSAGE at the end of the minibuffer.
+The text is displayed for `minibuffer-message-clear-timeout' seconds
+(if the value is a number), or until the next input event arrives,
+whichever comes first."
+  (when (and (not noninteractive)
+             (window-live-p (active-minibuffer-window)))
+    (with-current-buffer (window-buffer (active-minibuffer-window))
+      (setq message (if (string-match-p "\\` *\\[.+\\]\\'" message)
+                        ;; Make sure we can put-text-property.
+                        (copy-sequence message)
+                      (concat " [" message "]")))
+      (unless (or (null minibuffer-message-properties)
+                  ;; Don't overwrite the face properties the caller has set
+                  (text-properties-at 0 message))
+        (setq message (apply #'propertize message minibuffer-message-properties)))
+
+      (when (timerp minibuffer-message-timer)
+        (cancel-timer minibuffer-message-timer)
+        (setq minibuffer-message-timer nil))
+      (when (overlayp minibuffer-message-overlay)
+        (delete-overlay minibuffer-message-overlay)
+        (setq minibuffer-message-overlay nil))
+
+      (setq minibuffer-message-overlay
+            (make-overlay (point-max) (point-max) nil t t))
+      (unless (zerop (length 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))
+      (overlay-put minibuffer-message-overlay 'after-string message)
+
+      (when (numberp minibuffer-message-clear-timeout)
+        (setq minibuffer-message-timer
+              (run-with-timer minibuffer-message-clear-timeout nil
+                              (lambda ()
+                                (when (overlayp minibuffer-message-overlay)
+                                  (delete-overlay minibuffer-message-overlay)
+                                  (setq minibuffer-message-overlay nil)
+                                  (setq minibuffer-message-timer nil))))))
+
+      t)))
+
+(setq set-message-function 'set-minibuffer-message)
+
+(defun clear-minibuffer-message ()
+  "Clear minibuffer message."
+  (when (not noninteractive)
+    (when (timerp minibuffer-message-timer)
+      (cancel-timer minibuffer-message-timer)
+      (setq minibuffer-message-timer nil))
+    (when (overlayp minibuffer-message-overlay)
+      (delete-overlay minibuffer-message-overlay)
+      (setq minibuffer-message-overlay nil))))
+
+(setq clear-message-function 'clear-minibuffer-message)
+
 (defun minibuffer-completion-contents ()
   "Return the user input in a minibuffer before point as a string.
 In Emacs 22, that was what completion commands operated on.
diff --git a/src/keyboard.c b/src/keyboard.c
index 5135fd0bc8..5b1b6f2c95 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2990,6 +2990,8 @@ read_char (int commandflag, Lisp_Object map,
 	  safe_run_hooks (Qecho_area_clear_hook);
 	  clear_message (1, 0);
 	}
+      else if (FUNCTIONP (Vclear_message_function))
+        message1 (0);
     }
 
  reread_for_input_method:
diff --git a/src/xdisp.c b/src/xdisp.c
index 08c6927052..3d232d8e92 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -11706,13 +11706,32 @@ truncate_message_1 (ptrdiff_t nchars, Lisp_Object a2)
 static void
 set_message (Lisp_Object string)
 {
+  Lisp_Object message = Qnil;
+
   eassert (STRINGP (string));
 
-  message_enable_multibyte = STRING_MULTIBYTE (string);
+  if (FUNCTIONP (Vset_message_function))
+    {
+      ptrdiff_t count = SPECPDL_INDEX ();
+      specbind (Qinhibit_quit, Qt);
+      message = safe_call1 (Vset_message_function, string);
+      unbind_to (count, Qnil);
 
-  with_echo_area_buffer (0, -1, set_message_1, 0, string);
-  message_buf_print = false;
-  help_echo_showing_p = false;
+      if (STRINGP (message))
+        {
+          string = message;
+          message = Qnil;
+        }
+    }
+
+  if (NILP (message))
+    {
+      message_enable_multibyte = STRING_MULTIBYTE (string);
+
+      with_echo_area_buffer (0, -1, set_message_1, 0, string);
+      message_buf_print = false;
+      help_echo_showing_p = false;
+    }
 
   if (STRINGP (Vdebug_on_message)
       && STRINGP (string)
@@ -11768,6 +11787,14 @@ clear_message (bool current_p, bool last_displayed_p)
     {
       echo_area_buffer[0] = Qnil;
       message_cleared_p = true;
+
+      if (FUNCTIONP (Vclear_message_function))
+        {
+          ptrdiff_t count = SPECPDL_INDEX ();
+          specbind (Qinhibit_quit, Qt);
+          safe_call (1, Vclear_message_function);
+          unbind_to (count, Qnil);
+        }
     }
 
   if (last_displayed_p)
@@ -34940,6 +34967,14 @@ syms_of_xdisp (void)
 	       doc: /* If non-nil, debug if a message matching this regexp is displayed.  */);
   Vdebug_on_message = Qnil;
 
+  DEFVAR_LISP ("set-message-function", Vset_message_function,
+	       doc: /* If non-nil, function to set message.  */);
+  Vset_message_function = Qnil;
+
+  DEFVAR_LISP ("clear-message-function", Vclear_message_function,
+	       doc: /* If non-nil, function to clear message.  */);
+  Vclear_message_function = Qnil;
+
   DEFVAR_LISP ("redisplay--all-windows-cause", Vredisplay__all_windows_cause,
 	       doc: /*  */);
   Vredisplay__all_windows_cause = Fmake_hash_table (0, NULL);

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 20.12.2019 0:16, Juri Linkov wrote:
> +(defcustom minibuffer-message-clear-timeout 2

Hey now, shouldn't this be nil by default?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> +(defcustom minibuffer-message-clear-timeout 2
>
> Hey now, shouldn't this be nil by default?

But Eli said:

  I won't object as long as the default for Emacs 27 is as in the 2nd
  variant.

where the 2nd variant was:

  after-input=t   after-timeout=2 - clear either on input or after timeout

Or was it zero-based numbering meaning the 3rd variant on 1-based numbering?

I'm fine with either default value.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  38457@debbugs.gnu.org
> Date: Fri, 20 Dec 2019 01:17:38 +0200
> 
> >> +(defcustom minibuffer-message-clear-timeout 2
> >
> > Hey now, shouldn't this be nil by default?
> 
> But Eli said:
> 
>   I won't object as long as the default for Emacs 27 is as in the 2nd
>   variant.
> 
> where the 2nd variant was:
> 
>   after-input=t   after-timeout=2 - clear either on input or after timeout

Oops, I'm so sorry.  I meant the 3rd variant, of course.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

>> I think leaving "M-x" (or any other prompt) unobscured in this
>> situation is a nice benefit, and if it simplifies the code, it's even
>> more desirable.
>
> Implemented in a new patch.

This patch now makes easy to implement message stacking that solves the
issue of several successive calls to message resulting in the user only
seeing the last message.  This problem was raised many times, and now
with a function consisting of 12 lines of code attached below, such
messages are never lost anymore.

It's eyebrow-raising how many messages were missed before.
For example, on startup now it shows 3 lines of messages
I've never seen before:

  Desktop: 1 frame, 19 buffers restored.
  15.095076566 seconds
  Package cl is deprecated

I've only seen the last message "Package cl is deprecated" sometimes
when it was not overwritten by another later message.
Now all accumulated messages are displayed together.

It also allows noticing error messages never seen before.
In Gnus it now shows these lines together, not only the last message:

  Error while decoding: (args-out-of-range  *temp*-306008 1 4346)
  Error while decoding: (args-out-of-range  *temp* 1 7357)
  Fontifying...done

This code depends on the previous patch:


[-- Attachment #2: messages-stack.el --]
[-- Type: application/emacs-lisp, Size: 1067 bytes --]

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Fri, 20 Dec 2019 00:52:38 +0200
> 
> This patch now makes easy to implement message stacking that solves the
> issue of several successive calls to message resulting in the user only
> seeing the last message.  This problem was raised many times, and now
> with a function consisting of 12 lines of code attached below, such
> messages are never lost anymore.

Not sure I understand how this will work in practice, UX-wise.  Will
the mini-window be resized to show several lines of messages?  If so,
did you try this with resize-mini-windows set to nil?

> It's eyebrow-raising how many messages were missed before.
> For example, on startup now it shows 3 lines of messages
> I've never seen before:
> 
>   Desktop: 1 frame, 19 buffers restored.
>   15.095076566 seconds
>   Package cl is deprecated

I guess people who have many things happen at startup will like this
feature.  But I'm not sure we should enable this by default in Emacs
27.  WDYT?

> (defcustom messages-stack-timout 2
                            ^^^^^^
"timeout"

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> This patch now makes easy to implement message stacking that solves the
>> issue of several successive calls to message resulting in the user only
>> seeing the last message.  This problem was raised many times, and now
>> with a function consisting of 12 lines of code attached below, such
>> messages are never lost anymore.
>
> Not sure I understand how this will work in practice, UX-wise.  Will
> the mini-window be resized to show several lines of messages?  If so,
> did you try this with resize-mini-windows set to nil?

Actually, it works well only when resize-mini-windows is 'grow-only'.

>> It's eyebrow-raising how many messages were missed before.
>> For example, on startup now it shows 3 lines of messages
>> I've never seen before:
>> 
>>   Desktop: 1 frame, 19 buffers restored.
>>   15.095076566 seconds
>>   Package cl is deprecated
>
> I guess people who have many things happen at startup will like this
> feature.  But I'm not sure we should enable this by default in Emacs
> 27.  WDYT?

I posted a request for comments on emacs-devel.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Fri, 20 Dec 2019 00:16:46 +0200
> 
> So this patch implements only the option clear-timeout
> with the 2nd variant by default.

I'm very sorry to have misled you: I meant the 3rd variant, of course.
IOW, the default value of minibuffer-message-clear-timeout should not
be a number.

Other than that, the patch LGTM.  Please push, and thanks again for
all your work on these matters.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

tags 38457 fixed
close 38457 27.0.50
quit

> IOW, the default value of minibuffer-message-clear-timeout should not
> be a number.
>
> Other than that, the patch LGTM.  Please push, and thanks again for
> all your work on these matters.

Pushed with the nil default of minibuffer-message-clear-timeout.
Also added documentation to docstrings and to the manual.
Please check if everything is alright.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 38457@debbugs.gnu.org
> Date: Sun, 22 Dec 2019 00:02:48 +0200
> 
> Pushed with the nil default of minibuffer-message-clear-timeout.
> Also added documentation to docstrings and to the manual.
> Please check if everything is alright.

Thanks, it's fine (I made some minor wording changes).

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

Hi Juri,

I like the new behavior quite a bit, and thanks for working on it.

Here's something that's a matter of taste, but:

On 20.12.2019 0:16, Juri Linkov wrote:
> +      (setq message (if (string-match-p "\\` *\\[.+\\]\\'" message)
> +                        ;; Make sure we can put-text-property.
> +                        (copy-sequence message)
> +                      (concat " [" message "]")))

I prefer how it looks without an extra space here. Meaning

   (concat "[" message "]")

instead of

   (concat " [" message "]")

. Apparently minibuffer contents always end with a space already?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> Here's something that's a matter of taste, but:
>
>> +      (setq message (if (string-match-p "\\` *\\[.+\\]\\'" message)
>> +                        ;; Make sure we can put-text-property.
>> +                        (copy-sequence message)
>> +                      (concat " [" message "]")))
>
> I prefer how it looks without an extra space here. Meaning
>
>   (concat "[" message "]")
>
> instead of
>
>   (concat " [" message "]")
>
> . Apparently minibuffer contents always end with a space already?

For compatibility with minibuffer-message this code was copied
from minibuffer-message.  And indeed there is only one space
between the minibuffer contents and the message.  If you see two spaces,
maybe this is due to misconfiguration?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 22.12.2019 0:03, Juri Linkov wrote:
> For compatibility with minibuffer-message this code was copied
> from minibuffer-message.  And indeed there is only one space
> between the minibuffer contents and the message.  If you see two spaces,
> maybe this is due to misconfiguration?

Seems you are right, sorry about that. I've pulled the latest master, 
rebuilt, and there are no extra spaces. Maybe it was caused by 
minimess.el in my init dir, IDK.

Here's something else to consider: if the user has resize-mini-windows 
set to nil, and the user has icomplete-mode on (for instance), the 
message might not be visible behind the (almost cut off) list of 
completions.

And even of resize-mini-windows has the default value, the message might 
appear so far from where the user is looking to be almost unnoticeable.

Should we try to reuse the area on the bottom-left's window mode-line? 
Like eldoc-minibuffer-message does.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

> Here's something else to consider: if the user has resize-mini-windows set
> to nil, and the user has icomplete-mode on (for instance), the message
> might not be visible behind the (almost cut off) list of completions.
>
> And even of resize-mini-windows has the default value, the message might
> appear so far from where the user is looking to be almost unnoticeable.

IDK, I tried to set resize-mini-windows to nil and in icomplete-mode
can see the message.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 24.12.2019 0:58, Juri Linkov wrote:
> IDK, I tried to set resize-mini-windows to nil and in icomplete-mode
> can see the message.

My bad, last time I was trying it was with ido-mode. There, the message 
comes at the end of all completions.

With icomplete, however, it's printed before the completions. And it 
also appears to the left of the cursor. Is that intentional?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>> IDK, I tried to set resize-mini-windows to nil and in icomplete-mode
>> can see the message.
>
> My bad, last time I was trying it was with ido-mode. There, the message
> comes at the end of all completions.
>
> With icomplete, however, it's printed before the completions. And it also
> appears to the left of the cursor. Is that intentional?

Code in set-minibuffer-message was copied from minibuffer-message
where handling of the cursor and after-string overlay was intentional.

Could ido-mode use an overlay the same way as it's used in icomplete
that plays nicely with the set-minibuffer-message overlay?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 25.12.2019 1:47, Juri Linkov wrote:
>>> IDK, I tried to set resize-mini-windows to nil and in icomplete-mode
>>> can see the message.
>>
>> My bad, last time I was trying it was with ido-mode. There, the message
>> comes at the end of all completions.
>>
>> With icomplete, however, it's printed before the completions. And it also
>> appears to the left of the cursor. Is that intentional?
> 
> Code in set-minibuffer-message was copied from minibuffer-message
> where handling of the cursor and after-string overlay was intentional.

Since after-string is used, shouldn't it always appear *after* the cursor?

> Could ido-mode use an overlay the same way as it's used in icomplete
> that plays nicely with the set-minibuffer-message overlay?

This seems to work, but it needs more testing:

diff --git a/lisp/ido.el b/lisp/ido.el
index 79f259b819..d47450fc40 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -4492,6 +4492,8 @@ ido-initiate-auto-merge
  	(ido-tidy))
        (throw 'ido contents))))

+(defvar ido--overlay nil)
+
  (defun ido-exhibit ()
    "Post command hook for Ido."
    ;; Find matching files and display a list in the minibuffer.
@@ -4726,7 +4728,12 @@ ido-exhibit
  	(let ((inf (ido-completions contents)))
  	  (setq ido-show-confirm-message nil)
  	  (ido-trace "inf" inf)
-	  (insert inf))
+          (when ido--overlay
+            (delete-overlay ido--overlay))
+          (let ((o (make-overlay (point-max) (point-max) nil t t)))
+            (when (> (length inf) 0)
+              (put-text-property 0 1 'cursor t inf))
+            (overlay-put o 'after-string inf)
+            (setq ido--overlay o)))
  	))))

  (defun ido-completions (name)

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 25 Dec 2019 18:30:57 +0200
> Cc: 38457@debbugs.gnu.org
> 
> > Code in set-minibuffer-message was copied from minibuffer-message
> > where handling of the cursor and after-string overlay was intentional.
> 
> Since after-string is used, shouldn't it always appear *after* the cursor?

No.  The cursor is always displayed after the overlay string.  If you
want it to do anything else, you need to put a 'cursor' property on
the overlay string.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 25.12.2019 18:44, Eli Zaretskii wrote:
> No.  The cursor is always displayed after the overlay string.  If you
> want it to do anything else, you need to put a 'cursor' property on
> the overlay string.

We do that there.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 25.12.2019 19:49, Dmitry Gutov wrote:
> On 25.12.2019 18:44, Eli Zaretskii wrote:
>> No.  The cursor is always displayed after the overlay string.  If you
>> want it to do anything else, you need to put a 'cursor' property on
>> the overlay string.
> 
> We do that there.

And yet, the message appears before the cursor in the described 
circumstances.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: 38457@debbugs.gnu.org, juri@linkov.net
> Date: Sat, 18 Jan 2020 03:59:31 +0300
> 
> On 25.12.2019 19:49, Dmitry Gutov wrote:
> > On 25.12.2019 18:44, Eli Zaretskii wrote:
> >> No.  The cursor is always displayed after the overlay string.  If you
> >> want it to do anything else, you need to put a 'cursor' property on
> >> the overlay string.
> > 
> > We do that there.
> 
> And yet, the message appears before the cursor in the described 
> circumstances.

Can I have a simple reproducer for this, so I could see what's going
on there?

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 18.01.2020 11:19, Eli Zaretskii wrote:

>>>> No.  The cursor is always displayed after the overlay string.  If you
>>>> want it to do anything else, you need to put a 'cursor' property on
>>>> the overlay string.
>>>
>>> We do that there.
>>
>> And yet, the message appears before the cursor in the described
>> circumstances.
> 
> Can I have a simple reproducer for this, so I could see what's going
> on there?

A couple scenarios:

1. M-x icomplete-mode
2. M-: (run-with-idle-timer 2 nil (lambda () (message "beep")))
2. C-h f
3. Input something that will lead to either [Matched] or [No Matches],
e.g. 'asd'.
4. See [beep] appear before the cursor.

OR

1. M-: (setq resize-mini-windows nil)
2. M-x icomplete-mode
(The same reproduces with Ido with my patch posted)
2. M-: (run-with-idle-timer 2 nil (lambda () (message "beep")))
3. C-h f
4. Input anything at all. E.g. something that will have matches, like 
'ft' or just 'f'.
5. See [beep] appear before the cursor.

Strangely, whether it appears before or after the cursor (at the very 
end of the minibuffer), is affected by the value of 
'resize-mini-windows' (???).

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> Cc: 38457@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 20 Jan 2020 15:30:06 +0300
> 
> On 18.01.2020 11:19, Eli Zaretskii wrote:
> 
> >>>> No.  The cursor is always displayed after the overlay string.  If you
> >>>> want it to do anything else, you need to put a 'cursor' property on
> >>>> the overlay string.
> >>>
> >>> We do that there.
> >>
> >> And yet, the message appears before the cursor in the described
> >> circumstances.
> > 
> > Can I have a simple reproducer for this, so I could see what's going
> > on there?
> 
> A couple scenarios:
> 
> 1. M-x icomplete-mode
> 2. M-: (run-with-idle-timer 2 nil (lambda () (message "beep")))
> 2. C-h f
> 3. Input something that will lead to either [Matched] or [No Matches],
> e.g. 'asd'.
> 4. See [beep] appear before the cursor.
> 
> OR
> 
> 1. M-: (setq resize-mini-windows nil)
> 2. M-x icomplete-mode
> (The same reproduces with Ido with my patch posted)
> 2. M-: (run-with-idle-timer 2 nil (lambda () (message "beep")))
> 3. C-h f
> 4. Input anything at all. E.g. something that will have matches, like 
> 'ft' or just 'f'.
> 5. See [beep] appear before the cursor.

Thanks.  For the record, I only see the problem if I type "C-h f f";
neither "C-h f asd" nor "C-h f ft" reproduce the issue, because the
list of the candidates displayed by Icomplete is too short.

> Strangely, whether it appears before or after the cursor (at the very 
> end of the minibuffer), is affected by the value of 
> 'resize-mini-windows' (???).

I think it is (was) just random, see below.

There were 2 separate problems here.  First, Icomplete displays the
completion candidates as an after-string, so we end up having 2
overlays at EOB, both of them with after-strings.  Question: which one
of them will be displayed first?  Answer: it isn't predictable, you
are at the mercy of the overlay-sorting order when all the criteria
for sorting compare equal.  So sometimes the "beep" thing is displayed
before the candidate list/"Not matched" and sometimes after.
Moreover, if it happens to be after, and resize-mini-windows is nil,
and the list of candidates is too long to be displayed in its
entirety, then "beep" will not be shown at all.

Solution: use overlay priority.  You will see that I gave the overlay
produced by set-minibuffer-message a very high priority.  But I'm not
wedded to that number, I just don't know what the likes of Ivy, Helm,
and other heavily customized environments could do in their completion
features.  If we can make the priority lower, say, 101, I'd be much
happier.  CC'ing Stefan who I hope will have some useful input on this
matter.

The second problem is with setting the cursor when we have several
overlays with after-strings one after another.  When this happens, it
is generally not enough to use the 'cursor' property of t on the
overlay string character where you want the cursor, you need to use a
number.  Which I did.  The reason is that overlay strings are
problematic in this case, because the code which sets the cursor
cannot know where the overlay start was in the buffer (unlike with
strings that come from display properties), so it needs more help, and
the integer value of the 'cursor' property provides that help.

I only tested the fix (now on the emacs-27 branch) with Icomplete, but
I think Ido will work correctly as well, if you use your patch there.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

On 21.01.2020 19:15, Eli Zaretskii wrote:
> Thanks.  For the record, I only see the problem if I type "C-h f f";
> neither "C-h f asd" nor "C-h f ft" reproduce the issue, because the
> list of the candidates displayed by Icomplete is too short.

Ah, that's right. I was using a small-ish window, one that Emacs opens 
by default. Nothing close to fullscreen.

> I think it is (was) just random, see below.

Hmm, guess so. Though it must have been a peculiar kind of random.

> Solution: use overlay priority.

Thank you, Eli.

I will push my Ido patch shortly as well.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Dmitry Gutov]

Eli, don't you think we should iron out this new feature's interaction 
with icomplete and ido before the release?

On 25.12.2019 19:30, Dmitry Gutov wrote:
> 
>> Could ido-mode use an overlay the same way as it's used in icomplete
>> that plays nicely with the set-minibuffer-message overlay?
> 
> This seems to work, but it needs more testing:
> 
> diff --git a/lisp/ido.el b/lisp/ido.el
> index 79f259b819..d47450fc40 100644
> --- a/lisp/ido.el
> +++ b/lisp/ido.el
> @@ -4492,6 +4492,8 @@ ido-initiate-auto-merge
>       (ido-tidy))
>         (throw 'ido contents))))
> 
> +(defvar ido--overlay nil)
> +
>   (defun ido-exhibit ()
>     "Post command hook for Ido."
>     ;; Find matching files and display a list in the minibuffer.
> @@ -4726,7 +4728,12 @@ ido-exhibit
>       (let ((inf (ido-completions contents)))
>         (setq ido-show-confirm-message nil)
>         (ido-trace "inf" inf)
> -      (insert inf))
> +          (when ido--overlay
> +            (delete-overlay ido--overlay))
> +          (let ((o (make-overlay (point-max) (point-max) nil t t)))
> +            (when (> (length inf) 0)
> +              (put-text-property 0 1 'cursor t inf))
> +            (overlay-put o 'after-string inf)
> +            (setq ido--overlay o)))
>       ))))
> 
>   (defun ido-completions (name)

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juri Linkov <juri@linkov.net>
>> 
>> > So please try to find a simpler fix for this problem, one that doesn't
>> > involve a timer.
>> 
>> This is the simplest and the safest solution.
>
> The simplest solution would be to prevent dabbrev-expand from
> displaying the message in the first place; there are conditions for
> that which evidently the new implementation somehow triggers.
>
>> I wanted to implement it earlier but had no time before.  I see no
>> problems with it.
>
> Let's not rock the boat too much so close to the release cycle because
> of a very minor problem which caused these changes.  You may not see
> any problems with it now, but I'm certain we will bump into numerous
> problems very soon if we make such significant changes in this
> infrastructure, which is used everywhere in Emacs.

Mmm, as you say, the message infrastructure is used everywhere in Emacs,
and it has received significant changes in aa89c84e00d.

I think the regression Stephen noticed is more widespread than just
dabbrev-expand: e.g. TRAMP is also affected[1].  Does it make sense to
address each symptom individually?  Were dabbrev, TRAMP, and possibly
others wrong to assume that they could call (message …) without causing
2-second delays?

NB: I am not saying that Juri's initial patch is the simplest nor the
safest.  It just seems to me that aa89c84e00d has already "rocked the
boat" quite a bit, and addressing this specific bug report by tweaking
dabbrev-expand will only take care of the tip of the iceberg, to
continue the nautical metaphor…


[1] C-x C-f /ssh:localhost: RET

    ⇒ This hangs for 10 seconds unless the user furiously spams some
    command (e.g. C-n), as each of the following message takes 2 seconds
    to disappear:

    > Tramp: Opening connection for localhost using ssh...
    > Tramp: Sending command ‘exec ssh   -o ControlMaster=auto -o ControlPath='tramp.%C' -o ControlPersist=no -e none localhost’
    > Tramp: Waiting for prompts from remote shell...done
    > Tramp: Found remote shell prompt on ‘localhost’
    > Tramp: Opening connection for localhost using ssh...done

    I haven't reported this as a separate issue yet, because I am
    assuming that both bugs are caused by recent changes in message and
    should be fixed there…

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>,  38457@debbugs.gnu.org,
>   stephen.berman@gmx.net
> Date: Thu, 05 Dec 2019 16:24:08 +0100
> 
> Mmm, as you say, the message infrastructure is used everywhere in Emacs,
> and it has received significant changes in aa89c84e00d.
> 
> I think the regression Stephen noticed is more widespread than just
> dabbrev-expand: e.g. TRAMP is also affected[1].  Does it make sense to
> address each symptom individually?

I didn't say individually, I said I would like to see a simpler
change, one that doesn't yet again change the infrastructure
significantly.

For example, to fix the dabbrev case we could bind
minibuffer-message-timeout to zero in progress-reporter-do-update.

> Were dabbrev, TRAMP, and possibly others wrong to assume that they
> could call (message …) without causing 2-second delays?

No.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Juri Linkov]

>>> > So please try to find a simpler fix for this problem, one that doesn't
>>> > involve a timer.
>>>
>>> This is the simplest and the safest solution.
>>
>> The simplest solution would be to prevent dabbrev-expand from
>> displaying the message in the first place; there are conditions for
>> that which evidently the new implementation somehow triggers.
>>
>>> I wanted to implement it earlier but had no time before.  I see no
>>> problems with it.
>>
>> Let's not rock the boat too much so close to the release cycle because
>> of a very minor problem which caused these changes.  You may not see
>> any problems with it now, but I'm certain we will bump into numerous
>> problems very soon if we make such significant changes in this
>> infrastructure, which is used everywhere in Emacs.
>
> Mmm, as you say, the message infrastructure is used everywhere in Emacs,
> and it has received significant changes in aa89c84e00d.
>
> I think the regression Stephen noticed is more widespread than just
> dabbrev-expand: e.g. TRAMP is also affected[1].

I know that TRAMP is affected.  I had no time to address this before, sorry,
but now this is fixed by the previous patch I sent 3 days ago that uses a timer.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  38457@debbugs.gnu.org,
>   stephen.berman@gmx.net
> Date: Fri, 06 Dec 2019 02:06:30 +0200
> 
> > I think the regression Stephen noticed is more widespread than just
> > dabbrev-expand: e.g. TRAMP is also affected[1].
> 
> I know that TRAMP is affected.  I had no time to address this before, sorry,
> but now this is fixed by the previous patch I sent 3 days ago that uses a timer.

We need to fix this without a timer, and we need to fix this before
the emacs-27 branch is cut.

One possibility is for 'message' to bind 'minibuffer-message-timeout'
to zero when it calls 'minibuffer-message'.  Would this work in all
the use cases discovered to day?

Thanks.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Kévin Le Gouguec]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> One possibility is for 'message' to bind 'minibuffer-message-timeout'
> to zero when it calls 'minibuffer-message'.  Would this work in all
> the use cases discovered to day?

FWIW the following patch does remove the delay before {the dabbrev
expansion shows up,TRAMP proceeds with each step}:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuffer-message-timeout.patch --]
[-- Type: text/x-diff, Size: 525 bytes --]

diff --git a/src/editfns.c b/src/editfns.c
index 72a9cdba7a..3ee00fcc1b 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2895,6 +2895,8 @@ accent (\\=`) and apostrophe (\\=') are special in the format; see
 
       /* Avoid possible recursion.  */
       specbind (Qmessage_in_echo_area, Qt);
+      /* Do not delay callers (bug#38457).  */
+      specbind (intern ("minibuffer-message-timeout"), INT_TO_INTEGER (0));
 
       record_unwind_current_buffer ();
       Fset_buffer (Fwindow_buffer (Fold_selected_window ()));

[-- Attachment #3: Type: text/plain, Size: 103 bytes --]


Though that makes each message flicker briefly and disappear before the
user has a chance to read it.

bug#38457: 27.0.50; dabbrev-expand regression due to message change

[Drew Adams]

> Actually, the more I think about this the less I like the idea of
> calling minibuffer-message instead of 'message' under some
> circumstances, any circumstances.  minibuffer-message is meant for
> very different use case: temporarily displaying a message for a short
> time.  The messages it displays are usually ephemeral and dispensable;
> if the user misses such a message, no big deal.
> 
> By contrast, 'message' is used for similar use cases, but also for
> radically different ones, including those where several related
> messages are displayed in quick succession (a notable example is
> "Foo..." followed by "Foo...done"), or where a message is left in the
> echo area indefinitely if no input event arrives.  Crucially, Lisp
> programs do not tell 'message' which use case is required; instead,
> the following calls to 'message' or other events produce the required
> effects.  To produce the same effect with minibuffer-message will
> require too many changes all over the place.
> 
> So I think it is simply wrong to call minibuffer-message instead of
> 'message'.  There are too many different behavior aspects.  Several
> examples were already given, including debug-on-message.  One other
> aspect that I just bumped into is recording messages in the *Messages*
> buffer: minibuffer-message never did that, until a recent (and
> undocumented) change, also related to attempts to fix messages
> overwriting y-or-n-p prompts, so now we have one more incompatible
> change in minibuffer-message's behavior waiting to bite us down the
> road.
> 
> Therefore, I suggest to take a step back and discuss a better solution
> for these problems.

+1

I couldn't agree more.  In fact, I said
all of that in earlier messages here.

> The original problem was, AFAIU, that various minibuffer prompts
> become obscured by echo-area display of messages.  So one possible
> solution is to modify the subroutines of 'message', e.g.,
> set_message_1, to detect the conditions of the minibuffer being
> active, and insert the contents of the minibuffer into the echo-area
> buffer before the message text. Does anyone see problems with this?

I don't think that's a great approach.  It's
not about whether the minibuffer is active.

As a useful first approximation for approaching
these problems, perhaps just assume that the
minibuffer is _always_ active.  There's always
the possibility that some minibuffer interaction
is in progress.

Now, how to deal with some other interaction
that (fairly) wants to jump in and interrupt?

That's the question - whether that's a report
from an async process or a new confirmation
dialog, or whatever.

In general, I think such other interactions
should be the focus of our fixes.  For any
such: should it maybe be modal?  Should it
maybe report elsewhere (e.g. in some popup
buffer)?  Does it need to be logged, and if
so should that maybe be elsewhere than in
`*Messages*'?

Focusing on reports by async operations and
dialogs (e.g. confirmation dialogs) that
either should be modal or should happen
otherwise than by simply reading a key/char,
would be a good start, I think.