GNU bug report logs - #68547
[PATCH] ; Fix 'mode-line-format-right-align' with ElDoc

Previous Next

To reply to this bug, email your comments to 68547 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Wed, 17 Jan 2024 20:44:04 +0100

[Message part 1 (text/plain, inline)]

Tags: patch

The new mode line right alignment requires setting `mode-line-format` to
a list that contains (as in `memq`) the symbol
`mode-line-format-right-align`.  This is a bit brittle, and currently
`eldoc-minibuffer-message` modifies `mode-line-format` in a way that
happens to break `mode-line-format-right-align`.  To see that, set
`mode-line-format` to '("" mode-line-format-right-align "foo bar") and
then type `M-: (list`.  Now ElDoc info appears on the mode line, but
"bar" is no longer visible.

This patch makes ElDoc modify `mode-line-format` in an equivalent way
that avoids messing with `mode-line-format-right-align`.

[0001-Fix-mode-line-format-right-align-with-ElDoc.patch (text/patch, attachment)]

Message #8 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>, João Távora
 <joaotavora <at> gmail.com>
Cc: 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 11:56:55 +0200

> Date: Wed, 17 Jan 2024 20:44:04 +0100
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Tags: patch
> 
> The new mode line right alignment requires setting `mode-line-format` to
> a list that contains (as in `memq`) the symbol
> `mode-line-format-right-align`.  This is a bit brittle, and currently
> `eldoc-minibuffer-message` modifies `mode-line-format` in a way that
> happens to break `mode-line-format-right-align`.  To see that, set
> `mode-line-format` to '("" mode-line-format-right-align "foo bar") and
> then type `M-: (list`.  Now ElDoc info appears on the mode line, but
> "bar" is no longer visible.
> 
> This patch makes ElDoc modify `mode-line-format` in an equivalent way
> that avoids messing with `mode-line-format-right-align`.

Thanks.

João, any objections or comments?

Message #11 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Eshel Yaron <me <at> eshelyaron.com>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 10:20:39 +0000

On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > Date: Wed, 17 Jan 2024 20:44:04 +0100
> > From:  Eshel Yaron via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> >
> > Tags: patch
> >
> > The new mode line right alignment requires setting `mode-line-format` to
> > a list that contains (as in `memq`) the symbol
> > `mode-line-format-right-align`.  This is a bit brittle, and currently
> > `eldoc-minibuffer-message` modifies `mode-line-format` in a way that
> > happens to break `mode-line-format-right-align`.  To see that, set
> > `mode-line-format` to '("" mode-line-format-right-align "foo bar") and
> > then type `M-: (list`.  Now ElDoc info appears on the mode line, but
> > "bar" is no longer visible.
> >
> > This patch makes ElDoc modify `mode-line-format` in an equivalent way
> > that avoids messing with `mode-line-format-right-align`.
>
> Thanks.
>
> João, any objections or comments?

I think it looks good.  I just think the patch is a little
too newline friendly, i.e. the if can probably fit in a single
line without reaching 80 columns and it'll make it easier to
read.

João

Message #14 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 11:58:25 +0100

[Message part 1 (text/plain, inline)]

João Távora <joaotavora <at> gmail.com> writes:

> On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>>
>> > Date: Wed, 17 Jan 2024 20:44:04 +0100
>> > From:  Eshel Yaron via "Bug reports for GNU Emacs,
>> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>> >
>> > Tags: patch
>> >
>> > The new mode line right alignment requires setting `mode-line-format` to
>> > a list that contains (as in `memq`) the symbol
>> > `mode-line-format-right-align`.  This is a bit brittle, and currently
>> > `eldoc-minibuffer-message` modifies `mode-line-format` in a way that
>> > happens to break `mode-line-format-right-align`.  To see that, set
>> > `mode-line-format` to '("" mode-line-format-right-align "foo bar") and
>> > then type `M-: (list`.  Now ElDoc info appears on the mode line, but
>> > "bar" is no longer visible.
>> >
>> > This patch makes ElDoc modify `mode-line-format` in an equivalent way
>> > that avoids messing with `mode-line-format-right-align`.
>>
>> Thanks.
>>
>> João, any objections or comments?
>
> I think it looks good.  I just think the patch is a little
> too newline friendly, i.e. the if can probably fit in a single
> line without reaching 80 columns and it'll make it easier to
> read.

Yes, it fits nicely.  See updated patch below.

[v2-0001-Fix-mode-line-format-right-align-with-ElDoc-Bug-6.patch (text/x-patch, attachment)]

Message #17 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 12:01:46 +0000

On Sat, Jan 20, 2024 at 10:58 AM Eshel Yaron <me <at> eshelyaron.com> wrote:
>
> João Távora <joaotavora <at> gmail.com> writes:
>
> > On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >>
> >> > Date: Wed, 17 Jan 2024 20:44:04 +0100
> >> > From:  Eshel Yaron via "Bug reports for GNU Emacs,
> >> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> >> >
> >> > Tags: patch
> >> >
> >> > The new mode line right alignment requires setting `mode-line-format` to
> >> > a list that contains (as in `memq`) the symbol
> >> > `mode-line-format-right-align`.  This is a bit brittle, and currently
> >> > `eldoc-minibuffer-message` modifies `mode-line-format` in a way that
> >> > happens to break `mode-line-format-right-align`.  To see that, set
> >> > `mode-line-format` to '("" mode-line-format-right-align "foo bar") and
> >> > then type `M-: (list`.  Now ElDoc info appears on the mode line, but
> >> > "bar" is no longer visible.
> >> >
> >> > This patch makes ElDoc modify `mode-line-format` in an equivalent way
> >> > that avoids messing with `mode-line-format-right-align`.
> >>
> >> Thanks.
> >>
> >> João, any objections or comments?
> >
> > I think it looks good.  I just think the patch is a little
> > too newline friendly, i.e. the if can probably fit in a single
> > line without reaching 80 columns and it'll make it easier to
> > read.
>
> Yes, it fits nicely.  See updated patch below.

Thanks.  I pushed it.  I'm going to look at this function though,
since I don't I love the logic of destroying any old buffer's
mode-line-format and not restoring after the minibuffer is
exited.

Message #20 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 13:55:21 +0000

On Sat, Jan 20, 2024 at 12:01 PM João Távora <joaotavora <at> gmail.com> wrote:
> Thanks.  I pushed it.  I'm going to look at this function though,
> since I don't I love the logic of destroying any old buffer's
> mode-line-format and not restoring after the minibuffer is
> exited.

While we're on the subject, can you give this patch a try Eshel?

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 912a7357ca7..78ce7ecd123 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -182,7 +182,7 @@ eldoc-current-idle-delay
   "Idle time delay currently in use by timer.
 This is used to determine if `eldoc-idle-delay' is changed by the user.")

-(defvar eldoc-message-function #'eldoc-minibuffer-message
+(defvar eldoc-message-function #'eldoc--minibuffer-message
   "The function used by `eldoc--message' to display messages.
 It should receive the same arguments as `message'.")

@@ -292,43 +292,42 @@ eldoc-schedule-timer
          (setq eldoc-current-idle-delay eldoc-idle-delay)
          (timer-set-idle-time eldoc-timer eldoc-idle-delay t))))

-(defvar eldoc-mode-line-string nil)
-(put 'eldoc-mode-line-string 'risky-local-variable t)
-
-(defun eldoc-minibuffer-message (format-string &rest args)
+(defvar eldoc--saved-mlf nil
+  "Saved `mode-line-format' used in `eldoc--minibuffer-message'.")
+(defun eldoc--minibuffer-message (format-string &rest args)
   "Display message specified by FORMAT-STRING and ARGS on the
mode-line as needed.
 This function displays the message produced by formatting ARGS
 with FORMAT-STRING on the mode line when the current buffer is a minibuffer.
 Otherwise, it displays the message like `message' would."
-  (if (or (bound-and-true-p edebug-mode) (minibufferp))
-      (progn
-        (add-hook 'post-command-hook #'eldoc-minibuffer--cleanup)
- (with-current-buffer
-     (window-buffer
-      (or (window-in-direction 'above (minibuffer-window))
- (minibuffer-selected-window)
- (get-largest-window)))
-          (when (and mode-line-format
-                     (not (and (listp mode-line-format)
-                               (assq 'eldoc-mode-line-string
mode-line-format))))
-     (setq mode-line-format
-                  (funcall
-                   (if (listp mode-line-format) #'append #'list)
-                   (list "" '(eldoc-mode-line-string
-       (" " eldoc-mode-line-string " ")))
-                   mode-line-format)))
-          (setq eldoc-mode-line-string
-                (when (stringp format-string)
-                  (apply #'format-message format-string args)))
-          (force-mode-line-update)))
-    (apply #'message format-string args)))
-
-(defun eldoc-minibuffer--cleanup ()
-  (unless (or (bound-and-true-p edebug-mode) (minibufferp))
-    (setq eldoc-mode-line-string nil
-          ;; https://debbugs.gnu.org/16920
-          eldoc-last-message nil)
-    (remove-hook 'post-command-hook #'eldoc-minibuffer--cleanup)))
+  (cond ((bound-and-true-p edebug-mode)
+         (eldoc--message-in-mode-line 'edebug-mode-hook format-string args))
+        ((minibufferp)
+         (eldoc--message-in-mode-line 'minibuffer-exit-hook
format-string args))
+        (t
+         (apply #'message format-string args))))
+
+(defun eldoc--message-in-mode-line (hook format-string args)
+  (with-current-buffer
+      (window-buffer
+       (or (window-in-direction 'above (minibuffer-window))
+           (minibuffer-selected-window)
+           (get-largest-window)))
+    (let ((buf (current-buffer)))
+      (cl-labels ((cleanup ()
+                    (with-current-buffer buf
+                      (remove-hook hook #'cleanup)
+                      (setq mode-line-format eldoc--saved-mlf
+                            eldoc--saved-mlf nil))))
+        (add-hook hook #'cleanup)
+        (setq eldoc--saved-mlf (or eldoc--saved-mlf mode-line-format))
+        (when format-string
+          (setq
+           mode-line-format
+           (funcall (if (listp eldoc--saved-mlf) #'cons #'list)
+                    (and format-string
+                         (apply #'format-message format-string args))
+                    eldoc--saved-mlf)))
+        (force-mode-line-update)))))

 (make-obsolete
  'eldoc-message "use `eldoc-documentation-functions' instead." "eldoc-1.1.0")



-- 
João Távora

Message #23 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 16:33:36 +0100

Hi João,

João Távora <joaotavora <at> gmail.com> writes:

>> I'm going to look at this function though, since I don't I love the
>> logic of destroying any old buffer's mode-line-format and not
>> restoring after the minibuffer is exited.

Agreed.

> While we're on the subject, can you give this patch a try Eshel?

Sure.  At first glance this looks good, but I ran into some issues.
First of all, looks like some lines in the patch got wrapped, so I
needed to tweak the patch manually a bit in order to apply it.

Then I tried the following (somewhat pathological) use case:

0. Setup: (keymap-global-set "C-x w a" #'windmove-swap-states-left)
1. Open two buffers in two windows side by side.
2. Say `M-: (car `, to show info in the mode line of the left window.
3. Without quitting the minibuffer, use `C-x o` to switch to the right
   window, followed by `C-x w a` to switch the buffers in the left and
   right windows.
4. Return to the minibuffer and type `nil) RET` or something like that
   to exit the minibuffer.
5. The `mode-line-format` of the left buffer is becomes nil, i.e. no mode line.

Shuffling `mode-line-format` around is really tricky :(

Message #26 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 18:08:35 +0000

On Sat, Jan 20, 2024 at 3:33 PM Eshel Yaron <me <at> eshelyaron.com> wrote:

> Then I tried the following (somewhat pathological) use case:
>
> 0. Setup: (keymap-global-set "C-x w a" #'windmove-swap-states-left)
> 1. Open two buffers in two windows side by side.
> 2. Say `M-: (car `, to show info in the mode line of the left window.
> 3. Without quitting the minibuffer, use `C-x o` to switch to the right
>    window, followed by `C-x w a` to switch the buffers in the left and
>    right windows.
> 4. Return to the minibuffer and type `nil) RET` or something like that
>    to exit the minibuffer.
> 5. The `mode-line-format` of the left buffer is becomes nil, i.e. no mode line.
>
> Shuffling `mode-line-format` around is really tricky :(

Indeed.  Your case is pathological, but not particularly hard to
trigger.  Given the consequences are somewhat dire (vanished mode-line)
, it should most definitely be handled.

Try this version, please.  Only difference is it uses a setq-local
for eldoc--saved-mlf instead of a setq.

Please give it as much testing as you can.

João

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 912a7357ca7..1ba4e6a006f 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -182,7 +182,7 @@ eldoc-current-idle-delay
   "Idle time delay currently in use by timer.
 This is used to determine if `eldoc-idle-delay' is changed by the user.")

-(defvar eldoc-message-function #'eldoc-minibuffer-message
+(defvar eldoc-message-function #'eldoc--minibuffer-message
   "The function used by `eldoc--message' to display messages.
 It should receive the same arguments as `message'.")

@@ -292,43 +292,42 @@ eldoc-schedule-timer
          (setq eldoc-current-idle-delay eldoc-idle-delay)
          (timer-set-idle-time eldoc-timer eldoc-idle-delay t))))

-(defvar eldoc-mode-line-string nil)
-(put 'eldoc-mode-line-string 'risky-local-variable t)
-
-(defun eldoc-minibuffer-message (format-string &rest args)
+(defvar eldoc--saved-mlf nil
+  "Saved `mode-line-format' used in `eldoc--minibuffer-message'.")
+(defun eldoc--minibuffer-message (format-string &rest args)
   "Display message specified by FORMAT-STRING and ARGS on the
mode-line as needed.
 This function displays the message produced by formatting ARGS
 with FORMAT-STRING on the mode line when the current buffer is a minibuffer.
 Otherwise, it displays the message like `message' would."
-  (if (or (bound-and-true-p edebug-mode) (minibufferp))
-      (progn
-        (add-hook 'post-command-hook #'eldoc-minibuffer--cleanup)
- (with-current-buffer
-     (window-buffer
-      (or (window-in-direction 'above (minibuffer-window))
- (minibuffer-selected-window)
- (get-largest-window)))
-          (when (and mode-line-format
-                     (not (and (listp mode-line-format)
-                               (assq 'eldoc-mode-line-string
mode-line-format))))
-     (setq mode-line-format
-                  (funcall
-                   (if (listp mode-line-format) #'append #'list)
-                   (list "" '(eldoc-mode-line-string
-       (" " eldoc-mode-line-string " ")))
-                   mode-line-format)))
-          (setq eldoc-mode-line-string
-                (when (stringp format-string)
-                  (apply #'format-message format-string args)))
-          (force-mode-line-update)))
-    (apply #'message format-string args)))
-
-(defun eldoc-minibuffer--cleanup ()
-  (unless (or (bound-and-true-p edebug-mode) (minibufferp))
-    (setq eldoc-mode-line-string nil
-          ;; https://debbugs.gnu.org/16920
-          eldoc-last-message nil)
-    (remove-hook 'post-command-hook #'eldoc-minibuffer--cleanup)))
+  (cond ((bound-and-true-p edebug-mode)
+         (eldoc--message-in-mode-line 'edebug-mode-hook format-string args))
+        ((minibufferp)
+         (eldoc--message-in-mode-line 'minibuffer-exit-hook
format-string args))
+        (t
+         (apply #'message format-string args))))
+
+(defun eldoc--message-in-mode-line (hook format-string args)
+  (with-current-buffer
+      (window-buffer
+       (or (window-in-direction 'above (minibuffer-window))
+           (minibuffer-selected-window)
+           (get-largest-window)))
+    (let ((buf (current-buffer)))
+      (cl-labels ((cleanup ()
+                    (with-current-buffer buf
+                      (remove-hook hook #'cleanup)
+                      (setq mode-line-format eldoc--saved-mlf
+                            eldoc--saved-mlf nil))))
+        (add-hook hook #'cleanup)
+        (setq-local eldoc--saved-mlf (or eldoc--saved-mlf mode-line-format))
+        (when format-string
+          (setq-local
+           mode-line-format
+           (funcall (if (listp eldoc--saved-mlf) #'cons #'list)
+                    (and format-string
+                         (apply #'format-message format-string args))
+                    eldoc--saved-mlf)))
+        (force-mode-line-update)))))

 (make-obsolete
  'eldoc-message "use `eldoc-documentation-functions' instead." "eldoc-1.1.0")



-- 
João Távora

Message #29 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: me <at> eshelyaron.com, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 20:16:22 +0200

> From: João Távora <joaotavora <at> gmail.com>
> Date: Sat, 20 Jan 2024 18:08:35 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
> 
> -(defun eldoc-minibuffer-message (format-string &rest args)
> +(defvar eldoc--saved-mlf nil
> +  "Saved `mode-line-format' used in `eldoc--minibuffer-message'.")
> +(defun eldoc--minibuffer-message (format-string &rest args)
>    "Display message specified by FORMAT-STRING and ARGS on the
> mode-line as needed.

Does this remove a function that existed before?  If so, please don't,
since that function's name didn't include "--", so it was not an
internal function.

(I also find the tendency of using internal functions as values of
variables that are clearly meant to be customized by modes to be
undesirable.  They are not really internal functions.)

Message #32 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: me <at> eshelyaron.com, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sat, 20 Jan 2024 21:12:22 +0000

On Sat, Jan 20, 2024 at 6:16 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: João Távora <joaotavora <at> gmail.com>
> > Date: Sat, 20 Jan 2024 18:08:35 +0000
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
> >
> > -(defun eldoc-minibuffer-message (format-string &rest args)
> > +(defvar eldoc--saved-mlf nil
> > +  "Saved `mode-line-format' used in `eldoc--minibuffer-message'.")
> > +(defun eldoc--minibuffer-message (format-string &rest args)
> >    "Display message specified by FORMAT-STRING and ARGS on the
> > mode-line as needed.
>
> Does this remove a function that existed before?  If so, please don't,
> since that function's name didn't include "--", so it was not an
> internal function.

The function eglot-minibuffer-message is a value for
eldoc-message-function and it's "external" indeed.  But not by
design rather by "status quo" -- i.e. because at the time it was
introduced, the '-- 'convention was not always observed.

The correct way to customize ElDoc's messaging outlet is to
set 'eldoc-message-function', the variable. That doesn't
require referencing the should-have-been-internal symbol.

> (I also find the tendency of using internal functions as values of
> variables that are clearly meant to be customized by modes to be
> undesirable.  They are not really internal functions.)

I see nothing wrong with it.

An internal symbol that designates a function means that:

1) you shouldn't call it, it's probably a bug if you do, and it
   unnecessarily constrains the development of the library.

2) you needn't refer to its value function value via (function <symbol>)
   or equivalent.

'eglot-minibuffer-message' verifies both.

1) The function is an alias to message except in certain situations
   where it will use the mode-line of a certain buffer chosen
   heuristically.  It is also a misnomer and the docstring

2) The recommended way to customize eldoc-message-function or
   other function-holding variables is not by 'setq' but rather
   'add-function'.  `setq` can be used if you intend to restore
   the old value.

But as you noted, because of this design mistake, I've noticed
a small number of modes do set it via 'setq'.  The correct way
would be to create an obsolete alias.  But if you don't want
to I won't insist, and we let this dirt persist.  There's
certainly much worse.

So Eshel, if you try my patch, remove the '--' from the
eldoc--minibuffer-message please.

João

Message #35 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: me <at> eshelyaron.com, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sun, 21 Jan 2024 07:18:59 +0200

> From: João Távora <joaotavora <at> gmail.com>
> Date: Sat, 20 Jan 2024 21:12:22 +0000
> Cc: me <at> eshelyaron.com, 68547 <at> debbugs.gnu.org
> 
> > Does this remove a function that existed before?  If so, please don't,
> > since that function's name didn't include "--", so it was not an
> > internal function.
> 
> The function eglot-minibuffer-message is a value for
> eldoc-message-function and it's "external" indeed.  But not by
> design rather by "status quo" -- i.e. because at the time it was
> introduced, the '-- 'convention was not always observed.
> 
> The correct way to customize ElDoc's messaging outlet is to
> set 'eldoc-message-function', the variable. That doesn't
> require referencing the should-have-been-internal symbol.

It does, if some Lisp program wants to reset the value back for some
reason.

> > (I also find the tendency of using internal functions as values of
> > variables that are clearly meant to be customized by modes to be
> > undesirable.  They are not really internal functions.)
> 
> I see nothing wrong with it.

What is wrong with it is that it's a possible value of a public
variable, so the value is basically public by definition.

> But as you noted, because of this design mistake, I've noticed
> a small number of modes do set it via 'setq'.  The correct way
> would be to create an obsolete alias.  But if you don't want
> to I won't insist, and we let this dirt persist.  There's
> certainly much worse.

Thanks, I prefer not to rename it.  Who knows what code out there
relies on it being available.

Message #38 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sun, 21 Jan 2024 09:34:31 +0100

Hi,

João Távora <joaotavora <at> gmail.com> writes:

> So Eshel, if you try my patch...

I tried your new patch, and it works well, AFAICT.  Two suggestions:

1. This patch omits a space that would appear before the info on the
   mode line.  I like that bit of padding, so I suggest restoring it :)

2. In `eldoc--message-in-mode-line`, `add-hook` and `remove-hook` can be
   called with non-nil LOCAL argument to further localize the effect,
   although I'm not sure that makes a real difference in practice.


Best,

Eshel

Message #41 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sun, 21 Jan 2024 08:52:33 +0000

On Sun, Jan 21, 2024 at 8:34 AM Eshel Yaron <me <at> eshelyaron.com> wrote:

> I tried your new patch, and it works well, AFAICT.  Two suggestions:
>
> 1. This patch omits a space that would appear before the info on the
>    mode line.  I like that bit of padding, so I suggest restoring it :)

OK.

> 2. In `eldoc--message-in-mode-line`, `add-hook` and `remove-hook` can be
>    called with non-nil LOCAL argument to further localize the effect,
>    although I'm not sure that makes a real difference in practice.

I tend to think it would break your pathological use cases of
shuffling buffers above the minibuffer while it is ongoing, no?
Two or more separate transient cleanup lambdas can appear
in those cases, no?

João

Message #44 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Sun, 21 Jan 2024 14:20:24 +0100

João Távora <joaotavora <at> gmail.com> writes:

> On Sun, Jan 21, 2024 at 8:34 AM Eshel Yaron <me <at> eshelyaron.com> wrote:
>
>> I tried your new patch, and it works well, AFAICT.  Two suggestions:
>>
>> 1. This patch omits a space that would appear before the info on the
>>    mode line.  I like that bit of padding, so I suggest restoring it :)
>
> OK.
>
>> 2. In `eldoc--message-in-mode-line`, `add-hook` and `remove-hook` can be
>>    called with non-nil LOCAL argument to further localize the effect,
>>    although I'm not sure that makes a real difference in practice.
>
> I tend to think it would break your pathological use cases of
> shuffling buffers above the minibuffer while it is ongoing, no?
> Two or more separate transient cleanup lambdas can appear
> in those cases, no?

Yes, you're right.  My suggestion was based on a wrong assumption that
these calls take place with the minibuffer as the current buffer.

If I say something like `(with-current-buffer minibuf ...)` around the
`add-hook` and `remove-hook` calls, and make these calls change the
local value of `minibuffer-exit-hook`, that seems to work nicely.  In
particular, this avoids calling the cleanup lambda when you exit a
nested minibuffer (e.g. `M-: (foo bar) M-x baz RET RET`).  WDYT?

Message #47 received at 68547 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>,
 68547 <at> debbugs.gnu.org
Subject: Re: bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
Date: Wed, 5 Mar 2025 19:31:01 -0800

João Távora <joaotavora <at> gmail.com> writes:

> On Sat, Jan 20, 2024 at 3:33 PM Eshel Yaron <me <at> eshelyaron.com> wrote:
>
>> Then I tried the following (somewhat pathological) use case:
>>
>> 0. Setup: (keymap-global-set "C-x w a" #'windmove-swap-states-left)
>> 1. Open two buffers in two windows side by side.
>> 2. Say `M-: (car `, to show info in the mode line of the left window.
>> 3. Without quitting the minibuffer, use `C-x o` to switch to the right
>>    window, followed by `C-x w a` to switch the buffers in the left and
>>    right windows.
>> 4. Return to the minibuffer and type `nil) RET` or something like that
>>    to exit the minibuffer.
>> 5. The `mode-line-format` of the left buffer is becomes nil, i.e. no mode line.
>>
>> Shuffling `mode-line-format` around is really tricky :(
>
> Indeed.  Your case is pathological, but not particularly hard to
> trigger.  Given the consequences are somewhat dire (vanished mode-line)
> , it should most definitely be handled.
>
> Try this version, please.  Only difference is it uses a setq-local
> for eldoc--saved-mlf instead of a setq.
>
> Please give it as much testing as you can.
>
> João

Was this installed?  If not, should it be?

> diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
> index 912a7357ca7..1ba4e6a006f 100644
> --- a/lisp/emacs-lisp/eldoc.el
> +++ b/lisp/emacs-lisp/eldoc.el
> @@ -182,7 +182,7 @@ eldoc-current-idle-delay
>    "Idle time delay currently in use by timer.
>  This is used to determine if `eldoc-idle-delay' is changed by the user.")
>
> -(defvar eldoc-message-function #'eldoc-minibuffer-message
> +(defvar eldoc-message-function #'eldoc--minibuffer-message
>    "The function used by `eldoc--message' to display messages.
>  It should receive the same arguments as `message'.")
>
> @@ -292,43 +292,42 @@ eldoc-schedule-timer
>           (setq eldoc-current-idle-delay eldoc-idle-delay)
>           (timer-set-idle-time eldoc-timer eldoc-idle-delay t))))
>
> -(defvar eldoc-mode-line-string nil)
> -(put 'eldoc-mode-line-string 'risky-local-variable t)
> -
> -(defun eldoc-minibuffer-message (format-string &rest args)
> +(defvar eldoc--saved-mlf nil
> +  "Saved `mode-line-format' used in `eldoc--minibuffer-message'.")
> +(defun eldoc--minibuffer-message (format-string &rest args)
>    "Display message specified by FORMAT-STRING and ARGS on the
> mode-line as needed.
>  This function displays the message produced by formatting ARGS
>  with FORMAT-STRING on the mode line when the current buffer is a minibuffer.
>  Otherwise, it displays the message like `message' would."
> -  (if (or (bound-and-true-p edebug-mode) (minibufferp))
> -      (progn
> -        (add-hook 'post-command-hook #'eldoc-minibuffer--cleanup)
> - (with-current-buffer
> -     (window-buffer
> -      (or (window-in-direction 'above (minibuffer-window))
> - (minibuffer-selected-window)
> - (get-largest-window)))
> -          (when (and mode-line-format
> -                     (not (and (listp mode-line-format)
> -                               (assq 'eldoc-mode-line-string
> mode-line-format))))
> -     (setq mode-line-format
> -                  (funcall
> -                   (if (listp mode-line-format) #'append #'list)
> -                   (list "" '(eldoc-mode-line-string
> -       (" " eldoc-mode-line-string " ")))
> -                   mode-line-format)))
> -          (setq eldoc-mode-line-string
> -                (when (stringp format-string)
> -                  (apply #'format-message format-string args)))
> -          (force-mode-line-update)))
> -    (apply #'message format-string args)))
> -
> -(defun eldoc-minibuffer--cleanup ()
> -  (unless (or (bound-and-true-p edebug-mode) (minibufferp))
> -    (setq eldoc-mode-line-string nil
> -          ;; https://debbugs.gnu.org/16920
> -          eldoc-last-message nil)
> -    (remove-hook 'post-command-hook #'eldoc-minibuffer--cleanup)))
> +  (cond ((bound-and-true-p edebug-mode)
> +         (eldoc--message-in-mode-line 'edebug-mode-hook format-string args))
> +        ((minibufferp)
> +         (eldoc--message-in-mode-line 'minibuffer-exit-hook
> format-string args))
> +        (t
> +         (apply #'message format-string args))))
> +
> +(defun eldoc--message-in-mode-line (hook format-string args)
> +  (with-current-buffer
> +      (window-buffer
> +       (or (window-in-direction 'above (minibuffer-window))
> +           (minibuffer-selected-window)
> +           (get-largest-window)))
> +    (let ((buf (current-buffer)))
> +      (cl-labels ((cleanup ()
> +                    (with-current-buffer buf
> +                      (remove-hook hook #'cleanup)
> +                      (setq mode-line-format eldoc--saved-mlf
> +                            eldoc--saved-mlf nil))))
> +        (add-hook hook #'cleanup)
> +        (setq-local eldoc--saved-mlf (or eldoc--saved-mlf mode-line-format))
> +        (when format-string
> +          (setq-local
> +           mode-line-format
> +           (funcall (if (listp eldoc--saved-mlf) #'cons #'list)
> +                    (and format-string
> +                         (apply #'format-message format-string args))
> +                    eldoc--saved-mlf)))
> +        (force-mode-line-update)))))
>
>  (make-obsolete
>   'eldoc-message "use `eldoc-documentation-functions' instead." "eldoc-1.1.0")

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.