bug#73472: [PATCH] Substitute quotation marks in Eglot messages

bug#73472: [PATCH] Substitute quotation marks in Eglot messages

[Thomas Voss via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 251 bytes --]

Hi all,

Another quick patch from myself.  This patch makes it so that the
diagnostics and interactive prompts from Eglot (such as the prompt you
iteract with when renaming a symbol) properly respect
‘text-quoting-style’.

-- 
— Thomas

[-- Attachment #2: 0001-Substitute-quotation-marks-in-Eglot-messages.patch --]
[-- Type: text/x-patch, Size: 1788 bytes --]

From 2038b983a751a2e26c2e8a0b8d55ade29fb394fd Mon Sep 17 00:00:00 2001
From: Thomas Voss <mail@thomasvoss.com>
Date: Wed, 25 Sep 2024 15:51:36 +0200
Subject: [PATCH] Substitute quotation marks in Eglot messages

* lisp/progmodes/eglot.el (eglot--error) (eglot--message): Wrap the
message in `substitute-quotes'.
* lisp/progmodes/eglot.el (eglot-rename): Format the prompt using
`format-prompt' as opposed to `format'.

Copyright-paperwork-exempt: yes
---
 lisp/progmodes/eglot.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index a358b82..ee52767 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1698,11 +1698,11 @@ eglot--connect
 ;;;
 (defun eglot--error (format &rest args)
   "Error out with FORMAT with ARGS."
-  (error "[eglot] %s" (apply #'format format args)))
+  (error "[eglot] %s" (substitute-quotes (apply #'format format args))))
 
 (defun eglot--message (format &rest args)
   "Message out with FORMAT with ARGS."
-  (message "[eglot] %s" (apply #'format format args)))
+  (message "[eglot] %s" (substitute-quotes (apply #'format format args))))
 
 (defun eglot--warn (format &rest args)
   "Warning message with FORMAT and ARGS."
@@ -3704,8 +3704,8 @@ eglot-rename
   "Rename the current symbol to NEWNAME."
   (interactive
    (list (read-from-minibuffer
-          (format "Rename `%s' to: " (or (thing-at-point 'symbol t)
-                                         "unknown symbol"))
+          (format-prompt "Rename `%s' to" nil (or (thing-at-point 'symbol t)
+                                                  "unknown symbol"))
           nil nil nil nil
           (symbol-name (symbol-at-point)))))
   (eglot-server-capable-or-lose :renameProvider)
-- 
2.46.2

bug#73472: [PATCH] Substitute quotation marks in Eglot messages

[Eli Zaretskii]

> Date: Wed, 25 Sep 2024 16:03:32 +0200
> From:  "Thomas Voss" via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Another quick patch from myself.  This patch makes it so that the
> diagnostics and interactive prompts from Eglot (such as the prompt you
> iteract with when renaming a symbol) properly respect
> ‘text-quoting-style’.

Thanks.

> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -1698,11 +1698,11 @@ eglot--connect
>  ;;;
>  (defun eglot--error (format &rest args)
>    "Error out with FORMAT with ARGS."
> -  (error "[eglot] %s" (apply #'format format args)))
> +  (error "[eglot] %s" (substitute-quotes (apply #'format format args))))
>  
>  (defun eglot--message (format &rest args)
>    "Message out with FORMAT with ARGS."
> -  (message "[eglot] %s" (apply #'format format args)))
> +  (message "[eglot] %s" (substitute-quotes (apply #'format format args))))
>  
>  (defun eglot--warn (format &rest args)
>    "Warning message with FORMAT and ARGS."
> @@ -3704,8 +3704,8 @@ eglot-rename
>    "Rename the current symbol to NEWNAME."
>    (interactive
>     (list (read-from-minibuffer
> -          (format "Rename `%s' to: " (or (thing-at-point 'symbol t)
> -                                         "unknown symbol"))
> +          (format-prompt "Rename `%s' to" nil (or (thing-at-point 'symbol t)
> +                                                  "unknown symbol"))
>            nil nil nil nil
>            (symbol-name (symbol-at-point)))))
>    (eglot-server-capable-or-lose :renameProvider)

Hmm... in the first two changes, how do we know that the quote
characters are indeed quoting stuff in the context where we substitute
quotes according to quoting-style?  Those functions get strings they
know nothing about, so it looks a bit risky to blindly convert any
quoting characters there.  Or what am I missing?

bug#73472: [PATCH] Substitute quotation marks in Eglot messages

[Thomas Voss via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Eli,

On Wed Sep 25, 2024 at 6:01 PM CEST, Eli Zaretskii wrote:
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -1698,11 +1698,11 @@ eglot--connect
> >  ;;;
> >  (defun eglot--error (format &rest args)
> >    "Error out with FORMAT with ARGS."
> > -  (error "[eglot] %s" (apply #'format format args)))
> > +  (error "[eglot] %s" (substitute-quotes (apply #'format format args))))
> >  
> >  (defun eglot--message (format &rest args)
> >    "Message out with FORMAT with ARGS."
> > -  (message "[eglot] %s" (apply #'format format args)))
> > +  (message "[eglot] %s" (substitute-quotes (apply #'format format args))))
> >  
> >  (defun eglot--warn (format &rest args)
> >    "Warning message with FORMAT and ARGS."
> > @@ -3704,8 +3704,8 @@ eglot-rename
> >    "Rename the current symbol to NEWNAME."
> >    (interactive
> >     (list (read-from-minibuffer
> > -          (format "Rename `%s' to: " (or (thing-at-point 'symbol t)
> > -                                         "unknown symbol"))
> > +          (format-prompt "Rename `%s' to" nil (or (thing-at-point 'symbol t)
> > +                                                  "unknown symbol"))
> >            nil nil nil nil
> >            (symbol-name (symbol-at-point)))))
> >    (eglot-server-capable-or-lose :renameProvider)
>
> Hmm... in the first two changes, how do we know that the quote
> characters are indeed quoting stuff in the context where we substitute
> quotes according to quoting-style?  Those functions get strings they
> know nothing about, so it looks a bit risky to blindly convert any
> quoting characters there.  Or what am I missing?

You’re right that it may be risky; I iterated through the usages of these
functions and nothing stood out to me as a potential location where some
unexpected behaviour might occur, although it is true that there is the
possibility.

Not sure why I didn’t think of this when creating the patch, but it’s
probably a much better idea to change this:

    (substitute-quotes (apply #'format format args))

to this:

    (apply #'format (substitute-quotes format) args)

This limits the scope of the substitution to just the format strings,
where we know no funny business is going on.  Unless you have any other
reservations I could submit a v2 of the patch to do that.

-- 
— Thomas

bug#73472: [PATCH] Substitute quotation marks in Eglot messages

[Eli Zaretskii]

> Date: Wed, 25 Sep 2024 20:03:20 +0200
> From: "Thomas Voss" <mail@thomasvoss.com>
> Cc: <73472@debbugs.gnu.org>
> 
> Not sure why I didn’t think of this when creating the patch, but it’s
> probably a much better idea to change this:
> 
>     (substitute-quotes (apply #'format format args))
> 
> to this:
> 
>     (apply #'format (substitute-quotes format) args)
> 
> This limits the scope of the substitution to just the format strings,
> where we know no funny business is going on.

That is of course better, but do we indeed know that format will never
use quote characters for something that is not quoting?

I think the caller should itself use substitute-quotes where
appropriate.

And I'd like to hear João's opinion as well.

bug#73472: [PATCH] Substitute quotation marks in Eglot messages

[João Távora]

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

On Thu, Sep 26, 2024, 06:52 Eli Zaretskii <eliz@gnu.org> wrote:

I think the caller should itself use substitute-quotes where
> appropriate.
>
> And I'd like to hear João's opinion as well.
>

I think I think so too. What Eglot format strings are affected by this
quote substitution? I.e. what problem is being solved here?

João

>

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