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.