unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
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))

  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

  List information: https://www.gnu.org/software/emacs/

* 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 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).