From: Dmitry Gutov <dmitry@gutov.dev>
To: Eli Zaretskii <eliz@gnu.org>
Cc: mjh@mjhoy.com, 67569@debbugs.gnu.org
Subject: bug#67569: 29.1; ruby-mode syntax highlighting breaks with variable named "index" and "/" operator
Date: Sat, 9 Dec 2023 19:38:28 +0200 [thread overview]
Message-ID: <c643698a-b90f-7368-94d0-54cc7b6e90c1@gutov.dev> (raw)
In-Reply-To: <83zfyjwiiy.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 591 bytes --]
On 09/12/2023 16:35, Eli Zaretskii wrote:
>> Date: Sat, 9 Dec 2023 16:13:02 +0200
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> Eli, should we put this in Emacs 29.2? It's not a regression (a fairly
>> old problem), but the fix looks simple enough.
> Yes, please.
Very good, pushed to emacs-29.
Here's a second patch on top which gets rid of the whitelist altogether
(the new heuristic seems to cover the cases better anyway), though I'm
on the fence whether this should go here or into master.
It seems to work well with some testing, but it is more adventurous than
the previous one.
[-- Attachment #2: ruby-parenless-regexp-no-whitelist.diff --]
[-- Type: text/x-patch, Size: 4263 bytes --]
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 0ecb3579278..862aaf8e182 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -2105,12 +2105,6 @@ ruby-find-library-file
"\\(%\\)[qQrswWxIi]?\\([[:punct:]]\\)"
"Regexp to match the beginning of percent literal.")
- (defconst ruby-syntax-methods-before-regexp
- '("gsub" "gsub!" "sub" "sub!" "scan" "split" "split!" "index" "match"
- "assert_match" "Given" "Then" "When")
- "Methods that can take regexp as the first argument.
-It will be properly highlighted even when the call omits parens.")
-
(defvar ruby-syntax-before-regexp-re
(concat
;; Special tokens that can't be followed by a division operator.
@@ -2122,11 +2116,9 @@ ruby-find-library-file
"\\|\\(?:^\\|\\s \\)"
(regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
"or" "not" "&&" "||"))
- ;; Method name from the list.
- "\\|\\_<"
- (regexp-opt ruby-syntax-methods-before-regexp t)
"\\)\\s *")
- "Regexp to match text that can be followed by a regular expression."))
+ "Regexp to match text that disambiguates a regular expression.
+Any slash character after any of these should begin a regexp."))
(defun ruby-syntax-propertize (start end)
"Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
@@ -2182,20 +2174,18 @@ ruby-syntax-propertize
(when (save-excursion
(forward-char -1)
(cl-evenp (skip-chars-backward "\\\\")))
- (let ((state (save-excursion (syntax-ppss (match-beginning 1))))
- division-like)
+ (let ((state (save-excursion (syntax-ppss (match-beginning 1)))))
(when (or
;; Beginning of a regexp.
(and (null (nth 8 state))
- (save-excursion
- (setq division-like
- (or (eql (char-after) ?\s)
- (not (eql (char-before (1- (point))) ?\s))))
- (forward-char -1)
- (looking-back ruby-syntax-before-regexp-re
- (line-beginning-position)))
- (not (and division-like
- (match-beginning 2))))
+ (or (not
+ ;; Looks like division.
+ (or (eql (char-after) ?\s)
+ (not (eql (char-before (1- (point))) ?\s))))
+ (save-excursion
+ (forward-char -1)
+ (looking-back ruby-syntax-before-regexp-re
+ (line-beginning-position)))))
;; End of regexp. We don't match the whole
;; regexp at once because it can have
;; string interpolation inside, or span
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index 81d0dfd75c9..a411b39a8fc 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -34,11 +34,11 @@ def foo
# Regexp after whitelisted method.
"abc".sub /b/, 'd'
-# Don't mismatch "sub" at the end of words.
-a = asub / aslb + bsub / bslb;
+# Don't mistake division for regexp.
+a = sub / aslb + bsub / bslb;
# Highlight the regexp after "if".
-x = toto / foo if /do bar/ =~ "dobar"
+x = toto / foo if / do bar/ =~ "dobar"
# Regexp options are highlighted.
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index a931541ba35..fea5f58b92e 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -164,7 +164,7 @@ ruby-slash-not-regexp-when-no-spaces
(ruby-assert-state "x = index/3" 3 nil))
(ert-deftest ruby-regexp-not-division-when-only-space-before ()
- (ruby-assert-state "x = index /3" 3 ?/))
+ (ruby-assert-state "x = foo_index /3" 3 ?/))
(ert-deftest ruby-slash-not-regexp-when-only-space-after ()
(ruby-assert-state "x = index/ 3" 3 nil))
next prev parent reply other threads:[~2023-12-09 17:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-01 14:27 bug#67569: 29.1; ruby-mode syntax highlighting breaks with variable named "index" and "/" operator Michael Hoy
2023-12-09 8:32 ` Eli Zaretskii
2023-12-16 11:58 ` Eli Zaretskii
2023-12-16 12:17 ` Dmitry Gutov
2023-12-09 14:13 ` Dmitry Gutov
2023-12-09 14:35 ` Eli Zaretskii
2023-12-09 17:38 ` Dmitry Gutov [this message]
2023-12-09 17:47 ` Eli Zaretskii
2023-12-09 18:00 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c643698a-b90f-7368-94d0-54cc7b6e90c1@gutov.dev \
--to=dmitry@gutov.dev \
--cc=67569@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=mjh@mjhoy.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.