all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
@ 2024-01-17 19:44 Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-20  9:56 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-17 19:44 UTC (permalink / raw)
  To: 68547

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

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`.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-mode-line-format-right-align-with-ElDoc.patch --]
[-- Type: text/patch, Size: 1376 bytes --]

From 5d8568e00c2c36ce2fbc7554635868826ec5009a Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Mon, 1 Jan 2024 22:14:59 +0100
Subject: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc

* lisp/emacs-lisp/eldoc.el (eldoc-minibuffer-message): Avoid nesting
'mode-line-format', since that breaks 'mode-line-format-right-align'.
---
 lisp/emacs-lisp/eldoc.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 4ee825136c9..a4faa25fd31 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -312,9 +312,13 @@ eldoc-minibuffer-message
                      (not (and (listp mode-line-format)
                                (assq 'eldoc-mode-line-string mode-line-format))))
 	    (setq mode-line-format
-		  (list "" '(eldoc-mode-line-string
-			     (" " eldoc-mode-line-string " "))
-			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)))
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-17 19:44 bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-20  9:56 ` Eli Zaretskii
  2024-01-20 10:20   ` João Távora
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2024-01-20  9:56 UTC (permalink / raw)
  To: Eshel Yaron, João Távora; +Cc: 68547

> 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@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?





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20  9:56 ` Eli Zaretskii
@ 2024-01-20 10:20   ` João Távora
  2024-01-20 10:58     ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 14+ messages in thread
From: João Távora @ 2024-01-20 10:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Eshel Yaron, 68547

On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz@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@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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 10:20   ` João Távora
@ 2024-01-20 10:58     ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-20 12:01       ` João Távora
  0 siblings, 1 reply; 14+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-20 10:58 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, 68547

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

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

> On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz@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@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Fix-mode-line-format-right-align-with-ElDoc-Bug-6.patch --]
[-- Type: text/x-patch, Size: 1343 bytes --]

From 9c69e20ed172a2bfa56056a62077ae1c68b9b979 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Mon, 1 Jan 2024 22:14:59 +0100
Subject: [PATCH v2] ; Fix 'mode-line-format-right-align' with ElDoc
 (Bug#68547)

* lisp/emacs-lisp/eldoc.el (eldoc-minibuffer-message): Avoid nesting
'mode-line-format', since that breaks 'mode-line-format-right-align'.
---
 lisp/emacs-lisp/eldoc.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 06970d40e8a..912a7357ca7 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -312,9 +312,11 @@ eldoc-minibuffer-message
                      (not (and (listp mode-line-format)
                                (assq 'eldoc-mode-line-string mode-line-format))))
 	    (setq mode-line-format
-		  (list "" '(eldoc-mode-line-string
-			     (" " eldoc-mode-line-string " "))
-			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)))
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 10:58     ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-20 12:01       ` João Távora
  2024-01-20 13:55         ` João Távora
  0 siblings, 1 reply; 14+ messages in thread
From: João Távora @ 2024-01-20 12:01 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: Eli Zaretskii, 68547

On Sat, Jan 20, 2024 at 10:58 AM Eshel Yaron <me@eshelyaron.com> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> > On Sat, Jan 20, 2024 at 9:57 AM Eli Zaretskii <eliz@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@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.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 12:01       ` João Távora
@ 2024-01-20 13:55         ` João Távora
  2024-01-20 15:33           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 14+ messages in thread
From: João Távora @ 2024-01-20 13:55 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: Eli Zaretskii, 68547

On Sat, Jan 20, 2024 at 12:01 PM João Távora <joaotavora@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





^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 13:55         ` João Távora
@ 2024-01-20 15:33           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-20 18:08             ` João Távora
  0 siblings, 1 reply; 14+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-20 15:33 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, 68547

Hi João,

João Távora <joaotavora@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 :(





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 15:33           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-20 18:08             ` João Távora
  2024-01-20 18:16               ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: João Távora @ 2024-01-20 18:08 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: Eli Zaretskii, 68547

On Sat, Jan 20, 2024 at 3:33 PM Eshel Yaron <me@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





^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 18:08             ` João Távora
@ 2024-01-20 18:16               ` Eli Zaretskii
  2024-01-20 21:12                 ` João Távora
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2024-01-20 18:16 UTC (permalink / raw)
  To: João Távora; +Cc: me, 68547

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 20 Jan 2024 18:08:35 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 68547@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.)





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 18:16               ` Eli Zaretskii
@ 2024-01-20 21:12                 ` João Távora
  2024-01-21  5:18                   ` Eli Zaretskii
  2024-01-21  8:34                   ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 14+ messages in thread
From: João Távora @ 2024-01-20 21:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: me, 68547

On Sat, Jan 20, 2024 at 6:16 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Sat, 20 Jan 2024 18:08:35 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, 68547@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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 21:12                 ` João Távora
@ 2024-01-21  5:18                   ` Eli Zaretskii
  2024-01-21  8:34                   ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2024-01-21  5:18 UTC (permalink / raw)
  To: João Távora; +Cc: me, 68547

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 20 Jan 2024 21:12:22 +0000
> Cc: me@eshelyaron.com, 68547@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.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-20 21:12                 ` João Távora
  2024-01-21  5:18                   ` Eli Zaretskii
@ 2024-01-21  8:34                   ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-01-21  8:52                     ` João Távora
  1 sibling, 1 reply; 14+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-21  8:34 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, 68547

Hi,

João Távora <joaotavora@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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-21  8:34                   ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-01-21  8:52                     ` João Távora
  2024-01-21 13:20                       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 14+ messages in thread
From: João Távora @ 2024-01-21  8:52 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: Eli Zaretskii, 68547

On Sun, Jan 21, 2024 at 8:34 AM Eshel Yaron <me@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





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc
  2024-01-21  8:52                     ` João Távora
@ 2024-01-21 13:20                       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 14+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-21 13:20 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, 68547

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

> On Sun, Jan 21, 2024 at 8:34 AM Eshel Yaron <me@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?





^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-01-21 13:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-17 19:44 bug#68547: [PATCH] ; Fix 'mode-line-format-right-align' with ElDoc Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-20  9:56 ` Eli Zaretskii
2024-01-20 10:20   ` João Távora
2024-01-20 10:58     ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-20 12:01       ` João Távora
2024-01-20 13:55         ` João Távora
2024-01-20 15:33           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-20 18:08             ` João Távora
2024-01-20 18:16               ` Eli Zaretskii
2024-01-20 21:12                 ` João Távora
2024-01-21  5:18                   ` Eli Zaretskii
2024-01-21  8:34                   ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-21  8:52                     ` João Távora
2024-01-21 13:20                       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.