* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
@ 2024-11-11 6:28 Roland Winkler
2024-11-14 8:11 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Roland Winkler @ 2024-11-11 6:28 UTC (permalink / raw)
To: 74307
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-11-11 6:28 bug#74307: 30.0.92; emacs-lisp font-locking word regexp Roland Winkler
@ 2024-11-14 8:11 ` Eli Zaretskii
2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Eli Zaretskii @ 2024-11-14 8:11 UTC (permalink / raw)
To: Roland Winkler, Stefan Monnier; +Cc: 74307
> 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.?
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-11-14 8:11 ` Eli Zaretskii
@ 2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-16 14:22 ` Eli Zaretskii
2024-11-14 16:49 ` Roland Winkler
2024-12-05 7:20 ` Juri Linkov
2 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-14 16:24 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 74307, Roland Winkler
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-16 14:22 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2024-11-16 14:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 74307-done, winkler
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-11-14 8:11 ` Eli Zaretskii
2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-14 16:49 ` Roland Winkler
2024-12-05 7:20 ` Juri Linkov
2 siblings, 0 replies; 9+ messages in thread
From: Roland Winkler @ 2024-11-14 16:49 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 74307, Stefan Monnier
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-11-14 8:11 ` Eli Zaretskii
2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-14 16:49 ` Roland Winkler
@ 2024-12-05 7:20 ` Juri Linkov
2024-12-05 7:38 ` Eli Zaretskii
2 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2024-12-05 7:20 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 74307, Roland Winkler, Stefan Monnier
>> (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 \\
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-12-05 7:20 ` Juri Linkov
@ 2024-12-05 7:38 ` Eli Zaretskii
2024-12-05 7:47 ` Juri Linkov
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2024-12-05 7:38 UTC (permalink / raw)
To: Juri Linkov; +Cc: 74307, winkler, monnier
> 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?
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-12-05 7:38 ` Eli Zaretskii
@ 2024-12-05 7:47 ` Juri Linkov
2024-12-05 8:04 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2024-12-05 7:47 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 74307, winkler, monnier
>> 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).
("\\(\\\\\\)\\([^\"\\]\\)"
^ permalink raw reply related [flat|nested] 9+ messages in thread* bug#74307: 30.0.92; emacs-lisp font-locking word regexp
2024-12-05 7:47 ` Juri Linkov
@ 2024-12-05 8:04 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2024-12-05 8:04 UTC (permalink / raw)
To: Juri Linkov; +Cc: 74307, winkler, monnier
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-12-05 8:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-11 6:28 bug#74307: 30.0.92; emacs-lisp font-locking word regexp Roland Winkler
2024-11-14 8:11 ` Eli Zaretskii
2024-11-14 16:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-16 14:22 ` Eli Zaretskii
2024-11-14 16:49 ` Roland Winkler
2024-12-05 7:20 ` Juri Linkov
2024-12-05 7:38 ` Eli Zaretskii
2024-12-05 7:47 ` Juri Linkov
2024-12-05 8:04 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).