GNU bug report logs - #73472
[PATCH] Substitute quotation marks in Eglot messages

Previous Next

Full log

View this message in rfc822 format

From: "Thomas Voss" <mail <at> thomasvoss.com>
To: "Eli Zaretskii" <eliz <at> gnu.org>, João Távora <joaotavora <at> gmail.com>
Cc: 73472 <at> debbugs.gnu.org
Subject: bug#73472: [PATCH] Substitute quotation marks in Eglot messages
Date: Wed, 25 Sep 2024 20:03:20 +0200

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

Previous Next

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