bug#74307: 30.0.92; emacs-lisp font-locking word regexp

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Roland Winkler]

Starting from emacs -Q, put the following into a buffer with
emacs-lisp-mode

  (setq foo "\\<foo\\>")

The part "foo\\" of the string "\\<foo\\>" will get
font-lock-variable-name-face, which looks odd.

I believe, this is due to a clause in lisp-mode.el that says

         ;; Words inside \\[], \\<>, \\{} or \\`' tend to be for
         ;; `substitute-command-keys'.

But this assumption is not always correct, in particular if ">" is
preceded by "\\", which happens when constructing regexps.

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Eli Zaretskii]

> From: Roland Winkler <winkler@gnu.org>
> Date: Mon, 11 Nov 2024 00:28:34 -0600
> 
> Starting from emacs -Q, put the following into a buffer with
> emacs-lisp-mode
> 
>   (setq foo "\\<foo\\>")
> 
> The part "foo\\" of the string "\\<foo\\>" will get
> font-lock-variable-name-face, which looks odd.
> 
> I believe, this is due to a clause in lisp-mode.el that says
> 
>          ;; Words inside \\[], \\<>, \\{} or \\`' tend to be for
>          ;; `substitute-command-keys'.
> 
> But this assumption is not always correct, in particular if ">" is
> preceded by "\\", which happens when constructing regexps.

I believe you are saying that in

         (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) ">")
                          (seq "{" (group-n 1 lisp-mode-symbol) "}")))
          (1 font-lock-variable-name-face prepend))

we should use something like the below instead?

     (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
                      (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "}"))

And similarly for \\[] etc.?

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

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

> I believe you are saying that in
>
>          (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) ">")
>                           (seq "{" (group-n 1 lisp-mode-symbol) "}")))
>           (1 font-lock-variable-name-face prepend))
>
> we should use something like the below instead?
>
>      (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
>                       (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "}"))
>
> And similarly for \\[] etc.?

Sounds good to me.


        Stefan

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Roland Winkler <winkler@gnu.org>,  74307@debbugs.gnu.org
> Date: Thu, 14 Nov 2024 11:24:48 -0500
> 
> > I believe you are saying that in
> >
> >          (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) ">")
> >                           (seq "{" (group-n 1 lisp-mode-symbol) "}")))
> >           (1 font-lock-variable-name-face prepend))
> >
> > we should use something like the below instead?
> >
> >      (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
> >                       (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "}"))
> >
> > And similarly for \\[] etc.?
> 
> Sounds good to me.

Thanks, installed on master, and closing the bug.

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Roland Winkler]

On Thu, Nov 14 2024, Eli Zaretskii wrote:
> we should use something like the below instead?
>
>      (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
>                       (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "q}"))

Yes, thanks.  (This is my first real-world encounter with rx.  Otherwise
I would have proposed it myself.)

> And similarly for \\[] etc.?

I do not know in what context backslash-quoted right square brackets may
appear in regexps.  But certainly, they do not make sense in the context
of substitute-command-keys either.  So excluding here backslash-quoted
right square brackets is probably for the better, too.

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Juri Linkov]

>>   (setq foo "\\<foo\\>")
>> 
>> The part "foo\\" of the string "\\<foo\\>" will get
>> font-lock-variable-name-face, which looks odd.
>> 
>> I believe, this is due to a clause in lisp-mode.el that says
>> 
>>          ;; Words inside \\[], \\<>, \\{} or \\`' tend to be for
>>          ;; `substitute-command-keys'.
>> 
>> But this assumption is not always correct, in particular if ">" is
>> preceded by "\\", which happens when constructing regexps.
>
> I believe you are saying that in
>
>          (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) ">")
>                           (seq "{" (group-n 1 lisp-mode-symbol) "}")))
>           (1 font-lock-variable-name-face prepend))
>
> we should use something like the below instead?
>
>      (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
>                       (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "}"))

The problem is that this removes highlighting from the last character
because it doesn't get into the group:

  (rx (seq "[" (group-n 1 lisp-mode-symbol) (not "\\") "]"))
  => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+\\)[^\\]]"

A possible solution is to move (not "\\") inside the group:

  (rx (seq "[" (group-n 1 (seq lisp-mode-symbol (not "\\"))) "]"))
  => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+[^\\]\\)]"

But this removes highlighting completely from the reported case of
(setq foo "\\<foo\\>").  However, I guess it should not have highlighting
anyway because this is an incorrect syntax of `substitute-command-keys'
that should match only \\[], \\<>, \\{} or \\`' without the second \\

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Roland Winkler <winkler@gnu.org>,  Stefan Monnier
>  <monnier@iro.umontreal.ca>,  74307@debbugs.gnu.org
> Date: Thu, 05 Dec 2024 09:20:16 +0200
> 
> >>   (setq foo "\\<foo\\>")
> >> 
> >> The part "foo\\" of the string "\\<foo\\>" will get
> >> font-lock-variable-name-face, which looks odd.
> >> 
> >> I believe, this is due to a clause in lisp-mode.el that says
> >> 
> >>          ;; Words inside \\[], \\<>, \\{} or \\`' tend to be for
> >>          ;; `substitute-command-keys'.
> >> 
> >> But this assumption is not always correct, in particular if ">" is
> >> preceded by "\\", which happens when constructing regexps.
> >
> > I believe you are saying that in
> >
> >          (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) ">")
> >                           (seq "{" (group-n 1 lisp-mode-symbol) "}")))
> >           (1 font-lock-variable-name-face prepend))
> >
> > we should use something like the below instead?
> >
> >      (,(rx "\\\\" (or (seq "<" (group-n 1 lisp-mode-symbol) (not "\\\\") ">")
> >                       (seq "{" (group-n 1 lisp-mode-symbol) (not "\\\\") "}"))
> 
> The problem is that this removes highlighting from the last character
> because it doesn't get into the group:
> 
>   (rx (seq "[" (group-n 1 lisp-mode-symbol) (not "\\") "]"))
>   => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+\\)[^\\]]"
> 
> A possible solution is to move (not "\\") inside the group:
> 
>   (rx (seq "[" (group-n 1 (seq lisp-mode-symbol (not "\\"))) "]"))
>   => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+[^\\]\\)]"
> 
> But this removes highlighting completely from the reported case of
> (setq foo "\\<foo\\>").  However, I guess it should not have highlighting
> anyway because this is an incorrect syntax of `substitute-command-keys'
> that should match only \\[], \\<>, \\{} or \\`' without the second \\

Sorry, I don't understand: the change which was supposed to fix this
was already installed.  If you are saying it caused regressions, could
you please show a recipe for reproducing those regressions?

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Juri Linkov]

>> The problem is that this removes highlighting from the last character
>> because it doesn't get into the group:
>> 
>>   (rx (seq "[" (group-n 1 lisp-mode-symbol) (not "\\") "]"))
>>   => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+\\)[^\\]]"
>> 
>> A possible solution is to move (not "\\") inside the group:
>> 
>>   (rx (seq "[" (group-n 1 (seq lisp-mode-symbol (not "\\"))) "]"))
>>   => "\\[\\(?1:\\(?:\\w\\|\\s_\\|\\\\.\\)+[^\\]\\)]"
>> 
>> But this removes highlighting completely from the reported case of
>> (setq foo "\\<foo\\>").  However, I guess it should not have highlighting
>> anyway because this is an incorrect syntax of `substitute-command-keys'
>> that should match only \\[], \\<>, \\{} or \\`' without the second \\
>
> Sorry, I don't understand: the change which was supposed to fix this
> was already installed.

Ah, so (setq foo "\\<foo\\>") should not be highlighted.  Ok, then
indeed (not "\\") should be inside the group.

> If you are saying it caused regressions, could you please show
> a recipe for reproducing those regressions?

A recipe is to put the following two lines into a buffer with
emacs-lisp-mode:

  (setq foo "\\<foo\\>")
  (setq foo "\\<foo>")

The first foo should not be highlighted, the second currently is
highlighted partially without the last character.  Here is the fix:

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index 99980a44ddf..95fbae48bb6 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -491,16 +491,16 @@ lisp-mode--search-key
          ;; Words inside \\[], \\<>, \\{} or \\`' tend to be for
          ;; `substitute-command-keys'.
          (,(rx "\\\\" (or (seq "["
-                               (group-n 1 lisp-mode-symbol) (not "\\") "]")
+                               (group-n 1 (seq lisp-mode-symbol (not "\\"))) "]")
                           (seq "`" (group-n 1
                                      ;; allow multiple words, e.g. "C-x a"
                                      lisp-mode-symbol (* " " lisp-mode-symbol))
                                "'")))
           (1 font-lock-constant-face prepend))
          (,(rx "\\\\" (or (seq "<"
-                               (group-n 1 lisp-mode-symbol) (not "\\") ">")
+                               (group-n 1 (seq lisp-mode-symbol (not "\\"))) ">")
                           (seq "{"
-                               (group-n 1 lisp-mode-symbol) (not "\\") "}")))
+                               (group-n 1 (seq lisp-mode-symbol) (not "\\")) "}")))
           (1 font-lock-variable-name-face prepend))
          ;; Ineffective backslashes (typically in need of doubling).
          ("\\(\\\\\\)\\([^\"\\]\\)"

bug#74307: 30.0.92; emacs-lisp font-locking word regexp

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: winkler@gnu.org,  monnier@iro.umontreal.ca,  74307@debbugs.gnu.org
> Date: Thu, 05 Dec 2024 09:47:05 +0200
> 
> > Sorry, I don't understand: the change which was supposed to fix this
> > was already installed.
> 
> Ah, so (setq foo "\\<foo\\>") should not be highlighted.  Ok, then
> indeed (not "\\") should be inside the group.
> 
> > If you are saying it caused regressions, could you please show
> > a recipe for reproducing those regressions?
> 
> A recipe is to put the following two lines into a buffer with
> emacs-lisp-mode:
> 
>   (setq foo "\\<foo\\>")
>   (setq foo "\\<foo>")
> 
> The first foo should not be highlighted, the second currently is
> highlighted partially without the last character.  Here is the fix:

Thanks, please install on master.