all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dgutov@yandex.ru>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: 6286@debbugs.gnu.org
Subject: bug#6286: General delimited literals in ruby-mode patch
Date: Wed, 25 Apr 2012 07:03:05 +0400	[thread overview]
Message-ID: <4F976969.6050804@yandex.ru> (raw)
In-Reply-To: <jwvipgpkrli.fsf-monnier+emacs@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]

So, the patch.

On 24.04.2012 21:09, Stefan Monnier wrote:
> Here is what it does:
> - Split large regexp into more manageable chunks.

I didn't think to use match groups this way. Very nice.

> - During the split I saw that gsub/sub/split/scan were matched (for
>    regexp) without regards to what precedes them, so "asub / a + bsub / b"
>    was taken for a regexp.

This fix has uncovered another problem: "gsub", "gsub!", "sub", "sub!", 
"scan", "split", and "split!" are not special tokens, those are all 
methods on class String: http://www.ruby-doc.org/core-1.9.3/String.html

The original author just collected the methods most often used with 
regexps. And now this is broken: "abcdec".split /[be]/

One might argue that this isn't the most important use case, and that 
methods with arity > 1 are covered by the second rule (comma after), but 
5 of these 7 methods can be called with just 1 argument. So that would 
mean backward incompatibility.

> - I found a problem in your approach to handling Cucumber code.

I'm assuming you mean this:

x = toto / foo if /do bar/ =~ "dobar" # shortened version

We can add a constraint that "do" is followed by (optionally) |a, d, c| 
(block arguments), and then EOL, since do ... end syntax isn't usually 
used with one-liner blocks, especially not after a regexp argument.

Or we can revert the change and do it the original way.

I looked into how other editors deal with regular expressions in Ruby. 
Vim is whitespace-sensitive. In the example above, the highlighting 
depends on whether you put space before "foo" (so it highlights one or 
the other regexp-looking expression).

Textmate favors the whitelisting approach, like ruby-mode had pre-patch: 
http://www.ruby-forum.com/topic/170852

It has one benefit in that when you've typed the regexp, it's already 
highlighted, before you type the block keyword. Might feel more natural.

In this approach, we'd move the "hardcoded" list of special method names 
to a variable, so that users might customize it, per project.

What do you think?

And here's a patch for another issue (attached).

-- Dmitry

[-- Attachment #2: 0001-ruby-mode-Don-t-propertize-percent-literals-inside-s.patch --]
[-- Type: text/plain, Size: 4992 bytes --]

From 05d10742e01cc3dceb4f465695daee7cc42215d6 Mon Sep 17 00:00:00 2001
From: Dmitry Gutov <dgutov@yandex.ru>
Date: Wed, 25 Apr 2012 06:55:50 +0400
Subject: [PATCH] ruby-mode: Don't propertize percent literals inside strings

---
 lisp/progmodes/ruby-mode.el |   58 ++++++++++++++++++++++++-------------------
 test/indent/ruby.rb         |    3 +++
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 5d79437..9ed3879 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -1162,7 +1162,7 @@ See `add-log-current-defun-function'."
            (7 (prog1 "\"" (ruby-syntax-propertize-heredoc end))))
           ;; Handle percent literals: %w(), %q{}, etc.
           ("\\(?:^\\|[[ \t\n<+(,=]\\)\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
-           (1 (prog1 "|" (ruby-syntax-propertize-general-delimiters end)))))
+           (1 (ruby-syntax-propertize-general-delimiters end))))
          (point) end))
 
       (defun ruby-syntax-propertize-heredoc (limit)
@@ -1198,31 +1198,37 @@ See `add-log-current-defun-function'."
             (beginning-of-line))))
 
       (defun ruby-syntax-propertize-general-delimiters (limit)
-        (goto-char (match-beginning 2))
-        (let* ((op (char-after))
-               (ops (char-to-string op))
-               (cl (or (cdr (aref (syntax-table) op))
-                       (cdr (assoc op '((?< . ?>))))))
-               parse-sexp-lookup-properties)
-          (ignore-errors
-            (if cl
-                (progn  ; Paired delimiters.
-                  ;; Delimiter pairs of the same kind can be nested
-                  ;; inside the literal, as long as they are balanced.
-                  ;; Create syntax table that ignores other characters.
-                  (with-syntax-table (make-char-table 'syntax-table nil)
-                    (modify-syntax-entry op (concat "(" (char-to-string cl)))
-                    (modify-syntax-entry cl (concat ")" ops))
-                    (modify-syntax-entry ?\\ "\\")
-                    (save-restriction
-                      (narrow-to-region (point) limit)
-                      (forward-list))))  ; skip to the paired character
-              ;; Single character delimiter.
-              (re-search-forward (concat "[^\\]\\(?:\\\\\\\\\\)*"
-                                         (regexp-quote ops)) limit nil))
-            ;; If we reached here, the closing delimiter was found.
-            (put-text-property (1- (point)) (point)
-                               'syntax-table (string-to-syntax "|")))))
+        (goto-char (match-beginning 1)) ; When multiline, the beginning
+        (let ((state (syntax-ppss))     ; may already be propertized.
+              (syntax-value (string-to-syntax "|")))
+          ;; Move forward either way, to escape inf loop.
+          (goto-char (match-beginning 2))
+          (unless (nth 3 state) ; not inside a string
+            (let* ((op (char-after))
+                   (ops (char-to-string op))
+                   (cl (or (cdr (aref (syntax-table) op))
+                           (cdr (assoc op '((?< . ?>))))))
+                   parse-sexp-lookup-properties)
+              (ignore-errors
+                (if cl
+                    (progn ; Paired delimiters.
+                      ;; Delimiter pairs of the same kind can be nested
+                      ;; inside the literal, as long as they are balanced.
+                      ;; Create syntax table that ignores other characters.
+                      (with-syntax-table (make-char-table 'syntax-table nil)
+                        (modify-syntax-entry op (concat "(" (char-to-string cl)))
+                        (modify-syntax-entry cl (concat ")" ops))
+                        (modify-syntax-entry ?\\ "\\")
+                        (save-restriction
+                          (narrow-to-region (point) limit)
+                          (forward-list)))) ; skip to the paired character
+                  ;; Single character delimiter.
+                  (re-search-forward (concat "[^\\]\\(?:\\\\\\\\\\)*"
+                                             (regexp-quote ops)) limit nil))
+                ;; If we reached here, the closing delimiter was found.
+                (put-text-property (1- (point)) (point) 'syntax-table
+                                   syntax-value)))
+            syntax-value)))
       )
 
   ;; For Emacsen where syntax-propertize-rules is not (yet) available,
diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
index c4a747a..fe1986a 100644
--- a/test/indent/ruby.rb
+++ b/test/indent/ruby.rb
@@ -7,6 +7,9 @@ c = %w(foo
  baz)
 d = %!hello!
 
+# Don't propertize percent literals inside strings.
+"(%s, %s)" % [123, 456]
+
 # A "do" after a slash means that slash is not a division, but it doesn't imply
 # it's a regexp-ender, since it can be a regexp-starter instead!
 x = toto / foo; if /do bar/ then
-- 
1.7.10.msysgit.1


  reply	other threads:[~2012-04-25  3:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-07 23:32 General delimited literals in ruby-mode patch Dmitry Gutov
2012-02-10  0:42 ` Dmitry Gutov
2012-02-10  5:03   ` Dmitry Gutov
2012-04-24 17:09     ` bug#6286: " Stefan Monnier
2012-04-25  3:03       ` Dmitry Gutov [this message]
2012-04-24 15:43   ` Stefan Monnier
2012-04-24 23:46     ` Dmitry Gutov
2012-04-25 14:15       ` Stefan Monnier
     [not found]         ` <4F981DEA.6060700@yandex.ru>
     [not found]           ` <jwv4ns7iubj.fsf-monnier+emacs@gnu.org>
2012-04-28 20:20             ` Dmitry Gutov
2012-05-03  5:39             ` Dmitry Gutov
2012-08-14  3:56               ` Dmitry Gutov
2012-08-14 12:40                 ` Stefan Monnier
2012-08-14 17:46                   ` Dmitry Gutov
2012-08-15  2:33                     ` Stefan Monnier

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=4F976969.6050804@yandex.ru \
    --to=dgutov@yandex.ru \
    --cc=6286@debbugs.gnu.org \
    --cc=monnier@IRO.UMontreal.CA \
    /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.