* bug#6286: General delimited literals in ruby-mode patch
[not found] ` <87k43vecyt.fsf@yandex.ru>
@ 2012-04-24 15:43 ` Stefan Monnier
2012-04-24 23:46 ` Dmitry Gutov
[not found] ` <87ehu3mga2.fsf@yandex.ru>
1 sibling, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-04-24 15:43 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 6286
Hi, Dmitry,
> I wrote a patch that fixes all examples except the last one (which is a
> different issue), and also supports nesting delimiters of the same type
> inside the literal: %r(//([^/])*/)
Thank you very much.
> Do I submit the final patch here, or to the Ruby Redmine tracker?
Here is fine, thanks (tho sending it to the bug-report address rather
than to emacs-devel would have been even better in this case).
> As I understand, the upstream version is maintained to be compatible
> with earlier Emacs versions, and I'm using the
> syntax-propertize-function feature here.
The maintainership is sadly unclear, indeed. If someone could try to
bring the two versions closer (and keep them in sync) that would be very
appreciated. Maybe it is enough to bring changes from the Ruby code to
the Emacs code and not the other way around, but only if there's a clear
understanding that the Ruby version is "deprecated" and won't see new
development. Also, I'm not sure the Emacs version includes all the
functionality of the current Ruby code, so there's some work to do.
> Am I trying to be too clever with the syntax table in
> `ruby-syntax-propertize-general-delimiters'? I figured it's the
> easiest way to avoid reimplementing `scan-lists'.
It doesn't strike me as too clever (actually it looks like the exact
same trick I used in perl-mode).
> In general, if I have an undercooked patch, should I post a message
> here, or just attach it to the related bug?
Attaching it to the bug report is better, in general, unless the patch
ends up attacking a larger problem.
> Why is the case of (fboundp #'syntax-propertize-rules) being nil still
> being handled, by the way?
I tried to preserve backward compatibility, to ease up merging the
two versions.
> This ruby-mode is not compatible with Emacs 23 either way.
That might be: I have not verified that it is indeed
backward compatible, I just tried to avoid consciously
introducing incompatibilities.
> - (or (not (eq ?/ c))
> - (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
> + ;; not a regexp or general delimited literal
> + (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
Could you explain this part of your patch?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
[not found] ` <87ehu3mga2.fsf@yandex.ru>
@ 2012-04-24 17:09 ` Stefan Monnier
2012-04-25 3:03 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-04-24 17:09 UTC (permalink / raw)
To: 6286; +Cc: Dmitry Gutov
I've installed your 2 patches, plus the patch below.
Could you take a look at my patch? Here is what it does:
- Fix up commenting conventions at random places.
- Split large regexp into more manageable chunks.
- 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.
- I found a problem in your approach to handling Cucumber code.
I don't know Ruby and don't use it (I looked up
http://web.njit.edu/all_topics/Prog_Lang_Docs/html/ruby/syntax.html for
help).
BTW, is it really true that "%Q(hello (my) world)" is correct?
That web-page doesn't clearly mention such nesting.
Stefan
=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog 2012-04-24 16:00:08 +0000
+++ lisp/ChangeLog 2012-04-24 16:35:47 +0000
@@ -1,3 +1,10 @@
+2012-04-24 Stefan Monnier <monnier@iro.umontreal.ca>
+
+ * progmodes/ruby-mode.el: Simplify last change, and cleanup code.
+ (ruby-syntax-propertize-regexp): Remove.
+ (ruby-syntax-propertize-function): Split regexp into chunks.
+ Match following code directly.
+
2012-04-24 Dmitry Gutov <dgutov@yandex.ru>
* progmodes/ruby-mode.el: Handle Cucumber defs (bug#6286).
=== modified file 'lisp/progmodes/ruby-mode.el'
--- lisp/progmodes/ruby-mode.el 2012-04-24 16:00:08 +0000
+++ lisp/progmodes/ruby-mode.el 2012-04-24 16:42:22 +0000
@@ -784,7 +784,7 @@
(not (looking-at "[a-z_]"))))
(and (looking-at ruby-operator-re)
(not (ruby-special-char-p))
- ;; operator at the end of line
+ ;; Operator at the end of line.
(let ((c (char-after (point))))
(and
;; (or (null begin)
@@ -794,8 +794,9 @@
;; (not (or (eolp) (looking-at "#")
;; (and (eq (car (nth 1 state)) ?{)
;; (looking-at "|"))))))
- ;; not a regexp or general delimited literal
- (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
+ ;; Not a regexp or general delimited literal.
+ (null (nth 0 (ruby-parse-region (or begin parse-start)
+ (point))))
(or (not (eq ?| (char-after (point))))
(save-excursion
(or (eolp) (forward-char -1))
@@ -1110,6 +1111,8 @@
mlist)))))
(declare-function ruby-syntax-propertize-heredoc "ruby-mode" (limit))
+(declare-function ruby-syntax-general-delimiters-goto-beg "ruby-mode" ())
+(declare-function ruby-syntax-propertize-general-delimiters "ruby-mode" (limit))
(if (eval-when-compile (fboundp #'syntax-propertize-rules))
;; New code that works independently from font-lock.
@@ -1121,18 +1124,37 @@
(ruby-syntax-general-delimiters-goto-beg)
(funcall
(syntax-propertize-rules
- ;; #{ }, #$hoge, #@foo are not comments
+ ;; #{ }, #$hoge, #@foo are not comments.
("\\(#\\)[{$@]" (1 "."))
- ;; $' $" $` .... are variables
- ;; ?' ?" ?` are ascii codes
+ ;; $' $" $` .... are variables.
+ ;; ?' ?" ?` are ascii codes.
("\\([?$]\\)[#\"'`]"
(1 (unless (save-excursion
;; Not within a string.
(nth 3 (syntax-ppss (match-beginning 0))))
(string-to-syntax "\\"))))
- ;; regexps
- ("\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)\\(?:if\\|elsif\\|unless\\|while\\|until\\|when\\|and\\|or\\|&&\\|||\\)\\|g?sub!?\\|scan\\|split!?\\)?\\s *\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
- (2 (ruby-syntax-propertize-regexp)))
+ ;; Regexps: regexps are distinguished from division either because
+ ;; of the keyword/symbol before them, or because of the code
+ ;; following them.
+ ((concat
+ ;; Special tokens that can't be followed by a division operator.
+ "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
+ (regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
+ "or" "&&" "||"
+ "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
+ "\\)\\s *\\)?"
+ ;; The regular expression itself.
+ "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
+ ;; Special code that cannot follow a division operator.
+ ;; FIXME: Just because the second slash of "/foo/ do bar" can't
+ ;; be a division, doesn't mean it can't *start* a regexp, as in
+ ;; "x = toto/foo; if /do bar/".
+ "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
+ (2 (when (or (match-beginning 1) (match-beginning 4))
+ (string-to-syntax "\"/")))
+ (3 (if (or (match-beginning 1) (match-beginning 4))
+ (string-to-syntax "\"/")
+ (goto-char (match-end 2)))))
("^=en\\(d\\)\\_>" (1 "!"))
("^\\(=\\)begin\\_>" (1 "!"))
;; Handle here documents.
@@ -1143,21 +1165,6 @@
(1 (prog1 "|" (ruby-syntax-propertize-general-delimiters end)))))
(point) end))
- (defun ruby-syntax-propertize-regexp ()
- (let ((syn (string-to-syntax "\"/")))
- (goto-char (match-end 3))
- (if (or
- ;; after paren, comma, operator, control flow keyword,
- ;; or a method from hardcoded list
- (match-beginning 1)
- ;; followed by comma or block
- (looking-at "[imxo]*\\s *\\(?:,\\|\\<do\\>\\)"))
- (progn
- (put-text-property (1- (point)) (point)
- 'syntax-table syn)
- syn)
- (goto-char (match-end 2)))))
-
(defun ruby-syntax-propertize-heredoc (limit)
(let ((ppss (syntax-ppss))
(res '()))
@@ -1199,7 +1206,7 @@
parse-sexp-lookup-properties)
(ignore-errors
(if cl
- (progn ; paired delimiters
+ (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.
@@ -1210,10 +1217,10 @@
(save-restriction
(narrow-to-region (point) limit)
(forward-list)))) ; skip to the paired character
- ;; single character delimiter
+ ;; Single character delimiter.
(re-search-forward (concat "[^\\]\\(?:\\\\\\\\\\)*"
(regexp-quote ops)) limit nil))
- ;; if we reached here, the closing delimiter was found
+ ;; If we reached here, the closing delimiter was found.
(put-text-property (1- (point)) (point)
'syntax-table (string-to-syntax "|")))))
)
@@ -1260,7 +1267,7 @@
(4 (7 . ?/))
(6 (7 . ?/)))
("^=en\\(d\\)\\_>" 1 "!")
- ;; general delimited string
+ ;; General delimited string.
("\\(^\\|[[ \t\n<+(,=]\\)\\(%[xrqQwW]?\\([^<[{(a-zA-Z0-9 \n]\\)[^\n\\\\]*\\(\\\\.[^\n\\\\]*\\)*\\(\\3\\)\\)"
(3 "\"")
(5 "\""))
=== modified file 'test/ChangeLog'
--- test/ChangeLog 2012-04-11 03:24:26 +0000
+++ test/ChangeLog 2012-04-24 17:02:20 +0000
@@ -1,3 +1,7 @@
+2012-04-24 Stefan Monnier <monnier@iro.umontreal.ca>
+
+ * indent/ruby.rb: New file, to test new syntax-propertize code.
+
2012-04-11 Glenn Morris <rgm@gnu.org>
* automated/vc-bzr.el (vc-bzr-test-faulty-bzr-autoloads): New test.
=== added file 'test/indent/ruby.rb'
--- test/indent/ruby.rb 1970-01-01 00:00:00 +0000
+++ test/indent/ruby.rb 2012-04-24 17:00:30 +0000
@@ -0,0 +1,19 @@
+# Don't mis-match "sub" at the end of words.
+a = asub / aslb + bsub / bslb;
+
+b = %Q{This is a "string"}
+c = %w(foo
+ bar
+ baz)
+d = %!hello!
+
+# 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
+ toto = 1
+ end
+
+# Some Cucumber code:
+Given /toto/ do
+ print "hello"
+end
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-04-24 15:43 ` bug#6286: General delimited literals in ruby-mode patch Stefan Monnier
@ 2012-04-24 23:46 ` Dmitry Gutov
2012-04-25 14:15 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2012-04-24 23:46 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
Hi Stefan,
Thanks for the answers.
On 24.04.2012 19:43, Stefan Monnier wrote:
> The maintainership is sadly unclear, indeed. If someone could try to
> bring the two versions closer (and keep them in sync) that would be very
> appreciated. Maybe it is enough to bring changes from the Ruby code to
> the Emacs code and not the other way around, but only if there's a clear
> understanding that the Ruby version is "deprecated" and won't see new
> development. Also, I'm not sure the Emacs version includes all the
> functionality of the current Ruby code, so there's some work to do.
They seem to have a ticket for this, but the comments are all in
Japanese: http://bugs.ruby-lang.org/issues/5142
And Babelfish doesn't help much.
If nobody beats me to it, I'm going to ask on the mailing list later.
>> This ruby-mode is not compatible with Emacs 23 either way.
>
> That might be: I have not verified that it is indeed
> backward compatible, I just tried to avoid consciously
> introducing incompatibilities.
To be precise, when you load it in 23.3, it complains about prog-mode's
function definition being void.
I guess that means we don't need to worry about maintaining the "else"
branch when implementing something that requires `syntax-propertize-rules'?
>> - (or (not (eq ?/ c))
>> - (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
>> + ;; not a regexp or general delimited literal
>> + (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
>
> Could you explain this part of your patch?
That's a fix for indentation after percent literals delimited with
operator characters: %r-abc-. But you seem to have already worked that out.
> BTW, is it really true that "%Q(hello (my) world)" is correct?
> That web-page doesn't clearly mention such nesting.
Yes, it seems to be one of the more obscure features:
irb(main):002:0> %Q(hello [(my]) world)
=> "hello [(my]) world"
It's mentioned here: http://phrogz.net/programmingruby/language.html
(to be continued)
-- Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-04-24 17:09 ` Stefan Monnier
@ 2012-04-25 3:03 ` Dmitry Gutov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2012-04-25 3:03 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
[-- 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-04-24 23:46 ` Dmitry Gutov
@ 2012-04-25 14:15 ` Stefan Monnier
[not found] ` <4F981DEA.6060700@yandex.ru>
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-04-25 14:15 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 6286
> To be precise, when you load it in 23.3, it complains about prog-mode's
> function definition being void.
Ah, that should be easy to fix.
> I guess that means we don't need to worry about maintaining the "else"
> branch when implementing something that requires `syntax-propertize-rules'?
We don't have to improve that "else branch", no, but we do want to
preserve its functionality. So, what you did in your patch (move the
old font-lock-keywords pattern to the "else branch") was just right.
>>> - (or (not (eq ?/ c))
>>> - (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
>>> + ;; not a regexp or general delimited literal
>>> + (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
>>
>> Could you explain this part of your patch?
> That's a fix for indentation after percent literals delimited with operator
> characters: %r-abc-.
So could it be refined so as to check for a "%" char? I.e. instead of
removing the old (not (eq ?/ c)), replace it with (not (memq c '(?/ ?%)))?
> But you seem to have already worked that out.
I just assumed the hunk was related to the rest of the patch ;-)
>> BTW, is it really true that "%Q(hello (my) world)" is correct?
>> That web-page doesn't clearly mention such nesting.
> Yes, it seems to be one of the more obscure features:
> irb(main):002:0> %Q(hello [(my]) world)
> => "hello [(my]) world"
> It's mentioned here: http://phrogz.net/programmingruby/language.html
OK, good, thanks.
>> - 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
Aha!
> The original author just collected the methods most often used with
> regexps. And now this is broken: "abcdec".split /[be]/
Oops.
> 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.
And as we've seen the "check for a comma or a do-block afterwards" is
not a reliable method.
>> - 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.
Yes.
> 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.
But that just reduces the likelihood of a problem without eliminating it:
x = toto / foo if /do
bar/ =~ "dobar" # Shortened version.
still has the exact same problem.
> 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).
That sounds like a bad idea: if the / is a division, it's OK because you
can easily decide to add/remove the space as needed, but if that / is
for a regexp it's not as easy because adding/removing that space changes
the regexp.
Or is it also linked to the presence of a preceding space? That might
not be so bad, then. E.g. " / " is division but "/ ", " /", and "/"
is regexp.
> Textmate favors the whitelisting approach, like ruby-mode had pre-patch:
> http://www.ruby-forum.com/topic/170852
They mention a good point: since you can always use %r/.../ to make it
clear you're using a regexp, it's better to err on the side of division
when in doubt.
> 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.
Yes, it's a nice side-advantage.
> 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?
Sounds good.
> And here's a patch for another issue (attached).
Regarding that patch, I think that you should do it differently:
- nitpick: rather than goto-char+syntax-ppss, you can just do
(syntax-ppss (match-beginning 1)).
- leave the (prog1 "|" (ruby-syntax-propertize-general-delimiters end))
as is.
- instead change ruby-syntax-propertize-general-delimiters so that it
first checks syntax-ppss to make sure it's inside
a general delimiter. This way, if the %Q appeared within a string
(or a comment, BTW), you'll handle it right: even if you've put
a "|" syntax on the character, that syntax has no effect on
syntax-ppss if it appears within a string/comment.
- Once you've done that, you can get rid of
ruby-syntax-general-delimiters-goto-beg and call
ruby-syntax-propertize-general-delimiters instead. You'll notice that
ruby-syntax-propertize-heredoc follows that model.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
[not found] ` <jwv4ns7iubj.fsf-monnier+emacs@gnu.org>
@ 2012-04-28 20:20 ` Dmitry Gutov
2012-05-03 5:39 ` Dmitry Gutov
1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2012-04-28 20:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]
On 25.04.2012 22:00, Stefan Monnier wrote:
>>> So could it be refined so as to check for a "%" char? I.e. instead of
>>> removing the old (not (eq ?/ c)), replace it with (not (memq c '(?/ ?%)))?
>> No, 'c' here is the char at the end of the previous line (see comment above
>> "let"), and the closing delimiter can be any punctuation char.
>
> OK, then I don't understand what the code used to do. That's OK, I have
> plenty of better things to do anyway ;-)
AFAIU, it was just an optimization. '/' was the only binary operator to
have double syntactic meaning. If c is not it, we definitely have
continued expression, and can skip checking parse status.
>>> x = toto / foo if /do
>>> bar/ =~ "dobar" # Shortened version.
>>> still has the exact same problem.
>> Yes. But let's face it, seeing this code in the wild is not very likely.
>
> You might be right.
>
>> And it's as easy to change as // -> %r//.
>
> I'm not sure what change you're referring to: how should I change the
> above code to make it unambiguous that the first / is a division rather
> than a regexp?
You would move the whole regexp to the next line, or (if the constraint
I suggested is in place) replace the line break with "\n":
x == toto / foo if /do\nbar/ =~ "dobar"
Anyway, the second patch does away this this approach.
>>> Or is it also linked to the presence of a preceding space? That might
>>> not be so bad, then. E.g. " / " is division but "/ ", " /", and "/"
>>> is regexp.
>> The lack of preceding space can turn a regexp into division, but not the
>> other way around.
>
> So you're saying that " / ", "/", and "/ " are for division, whereas " /"
> is for regexps?
Yes, though not without exceptions. "if / boo /" and "if/boo/" are both
considered to contain regexps. As well as "/ boo /" when not after a word.
The algorithm is likely more complex, I'm just describing how it works
with specific examples.
>> That's one more reason to do this just like TextMate, though.
>
> You mean because the problem can be fixed on the user's side without
> changing the Ruby code? Yes, that's a clear advantage, especially when
> browsing other people's code.
If TextMate bundles are easily editable by user, I don't know about that
(they might be). I just meant that having the same level of syntax
support across editors is a good thing. So that if you have resort to
%r//, it's the same for the other team members. And vice versa.
I'm speaking mostly hypothetically, but the percent literals patch is in
fact indirect result of my colleague using this syntax in TextMate, with
unclosed paren inside.
>> I'm not exactly sure why, but percent literal starting within a comment is
>> already (not) propertized as expected.
>
> Not in all cases:
>
> x #= "tot %q/to"; =
> /toto;
>
> now add&remove a space on the second line; then do the same on the
> first line.
> The approach I suggested is based on years of experience ;-)
I wasn't arguing against the approach, just making an observation. :)
But it's always good to have a counter-example.
I did something close to what you described (patch 0001), but it didn't
seem proper to call the same function in two different situations
(before search and after search when we already have match data), so
there's still two of them.
[-- Attachment #2: 0001-lisp-progmodes-ruby-mode.el-Don-t-propertize-percent.patch --]
[-- Type: text/plain, Size: 8572 bytes --]
>From bebf7f22ef746e1d20a5cdd4312684e02f0222f9 Mon Sep 17 00:00:00 2001
From: Dmitry Gutov <dgutov@yandex.ru>
Date: Sat, 28 Apr 2012 19:12:13 +0400
Subject: [PATCH 1/2] * lisp/progmodes/ruby-mode.el: Don't propertize percent
literals in strings or comments (bug#6286)
Change all names and references to "general delimited literals" to "percent
literals", seems to be a shorter and more popular term.
ruby-percent-literal-beg-re: New constant.
(ruby-propertize-containing-percent-literal): Check the type of literal more
carefully. Only propertize it but not other syntax above the point.
(ruby-syntax-propertize-percent-literal): Only propertize when not inside basic
string or comment. When the literal is unclosed, leave the text after it
unpropertized.
---
lisp/progmodes/ruby-mode.el | 84 ++++++++++++++++++++++++-------------------
test/indent/ruby.rb | 7 ++++
2 files changed, 54 insertions(+), 37 deletions(-)
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 5d79437..432680f 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -794,7 +794,7 @@ and `\\' when preceded by `?'."
;; (not (or (eolp) (looking-at "#")
;; (and (eq (car (nth 1 state)) ?{)
;; (looking-at "|"))))))
- ;; Not a regexp or general delimited literal.
+ ;; Not a regexp or percent literal.
(null (nth 0 (ruby-parse-region (or begin parse-start)
(point))))
(or (not (eq ?| (char-after (point))))
@@ -1111,17 +1111,21 @@ See `add-log-current-defun-function'."
mlist)))))
(declare-function ruby-syntax-propertize-heredoc "ruby-mode" (limit))
-(declare-function ruby-syntax-general-delimiters-goto-beg "ruby-mode" ())
-(declare-function ruby-syntax-propertize-general-delimiters "ruby-mode" (limit))
+(declare-function ruby-propertize-containing-percent-literal "ruby-mode" (limit))
+(declare-function ruby-syntax-propertize-percent-literal "ruby-mode" (limit))
(if (eval-when-compile (fboundp #'syntax-propertize-rules))
;; New code that works independently from font-lock.
(progn
+ (defconst ruby-percent-literal-beg-re
+ "\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
+ "Regexp to match the beginning of percent literal.")
+
(defun ruby-syntax-propertize-function (start end)
"Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
(goto-char start)
(ruby-syntax-propertize-heredoc end)
- (ruby-syntax-general-delimiters-goto-beg)
+ (ruby-propertize-containing-percent-literal end)
(funcall
(syntax-propertize-rules
;; #{ }, #$hoge, #@foo are not comments.
@@ -1161,8 +1165,8 @@ See `add-log-current-defun-function'."
((concat ruby-here-doc-beg-re ".*\\(\n\\)")
(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)))))
+ ((concat "\\(?:^\\|[[ \t\n<+(,=]\\)" ruby-percent-literal-beg-re)
+ (1 (prog1 "|" (ruby-syntax-propertize-percent-literal end)))))
(point) end))
(defun ruby-syntax-propertize-heredoc (limit)
@@ -1189,40 +1193,46 @@ See `add-log-current-defun-function'."
;; inf-loop.
(if (< (point) start) (goto-char start))))))
- (defun ruby-syntax-general-delimiters-goto-beg ()
- (let ((state (syntax-ppss)))
- ;; Move to the start of the literal, in case it's multiline.
- ;; TODO: determine the literal type more reliably here?
+ (defun ruby-propertize-containing-percent-literal (limit)
+ (let ((state (syntax-ppss))
+ (start (point)))
+ ;; When already inside percent literal, re-propertize it.
(when (eq t (nth 3 state))
(goto-char (nth 8 state))
- (beginning-of-line))))
+ (when (looking-at ruby-percent-literal-beg-re)
+ (ruby-syntax-propertize-percent-literal limit))
+ (when (< (point) start) (goto-char start)))))
- (defun ruby-syntax-propertize-general-delimiters (limit)
+ (defun ruby-syntax-propertize-percent-literal (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 "|")))))
+ ;; Not inside a simple string or comment.
+ (when (eq t (nth 3 (syntax-ppss)))
+ (let* ((op (char-after))
+ (ops (char-to-string op))
+ (cl (or (cdr (aref (syntax-table) op))
+ (cdr (assoc op '((?< . ?>))))))
+ parse-sexp-lookup-properties)
+ (condition-case nil
+ (progn
+ (if cl ; 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))
+ ;; Found the closing delimiter.
+ (put-text-property (1- (point)) (point) 'syntax-table
+ (string-to-syntax "|")))
+ ;; Unclosed literal, leave the following text unpropertized.
+ ((scan-error search-failed) (goto-char limit))))))
)
;; For Emacsen where syntax-propertize-rules is not (yet) available,
@@ -1267,7 +1277,7 @@ This should only be called after matching against `ruby-here-doc-end-re'."
(4 (7 . ?/))
(6 (7 . ?/)))
("^=en\\(d\\)\\_>" 1 "!")
- ;; General delimited string.
+ ;; Percent literal.
("\\(^\\|[[ \t\n<+(,=]\\)\\(%[xrqQwW]?\\([^<[{(a-zA-Z0-9 \n]\\)[^\n\\\\]*\\(\\\\.[^\n\\\\]*\\)*\\(\\3\\)\\)"
(3 "\"")
(5 "\""))
diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
index c4a747a..7a9d123 100644
--- a/test/indent/ruby.rb
+++ b/test/indent/ruby.rb
@@ -7,6 +7,13 @@ c = %w(foo
baz)
d = %!hello!
+# Don't propertize percent literals inside strings.
+"(%s, %s)" % [123, 456]
+
+# Nor inside comments.
+x = # "tot %q/to"; =
+y = 2 / 3
+
# 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
[-- Attachment #3: 0002-lisp-progmodes-ruby-mode.el-Go-back-to-method-whitel.patch --]
[-- Type: text/plain, Size: 4769 bytes --]
>From ac047bcebe08a8f850e3a3400156316088fd9520 Mon Sep 17 00:00:00 2001
From: Dmitry Gutov <dgutov@yandex.ru>
Date: Sat, 28 Apr 2012 22:11:00 +0400
Subject: [PATCH 2/2] * lisp/progmodes/ruby-mode.el: Go back to method
whitelisting for regexps.
ruby-syntax-methods-before-regexp: New variable.
(ruby-syntax-propertize-function): Use it to recognize
regexps. Don't look at the text after regexp, just use
the old approach.
* test/indent/ruby.rb: Update examples, add a new one.
---
lisp/progmodes/ruby-mode.el | 38 ++++++++++++++++++++------------------
test/indent/ruby.rb | 25 +++++++++++++------------
2 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 432680f..14ce846 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -226,6 +226,13 @@ Also ignores spaces after parenthesis when 'space."
"Use `ruby-encoding-map' to set encoding magic comment if this is non-nil."
:type 'boolean :group 'ruby)
+(defcustom 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."
+ :group 'ruby)
+
;; Safe file variables
(put 'ruby-indent-tabs-mode 'safe-local-variable 'booleanp)
(put 'ruby-indent-level 'safe-local-variable 'integerp)
@@ -1137,28 +1144,23 @@ See `add-log-current-defun-function'."
;; Not within a string.
(nth 3 (syntax-ppss (match-beginning 0))))
(string-to-syntax "\\"))))
- ;; Regexps: regexps are distinguished from division either because
- ;; of the keyword/symbol before them, or because of the code
- ;; following them.
+ ;; Regexps: regexps are distinguished from division because
+ ;; of the keyword, symbol, or method name before them.
((concat
;; Special tokens that can't be followed by a division operator.
- "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
+ "\\(^\\|[[=(,~?:;<>]"
+ ;; Control flow keywords and operators following bol or whitespace.
+ "\\|\\(?:^\\|\\s \\)"
(regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
- "or" "&&" "||"
- "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
- "\\)\\s *\\)?"
+ "or" "not" "&&" "||"))
+ ;; Method name from the list.
+ "\\|\\_<"
+ (regexp-opt ruby-syntax-methods-before-regexp)
+ "\\)\\s *"
;; The regular expression itself.
- "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
- ;; Special code that cannot follow a division operator.
- ;; FIXME: Just because the second slash of "/foo/ do bar" can't
- ;; be a division, doesn't mean it can't *start* a regexp, as in
- ;; "x = toto/foo; if /do bar/".
- "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
- (2 (when (or (match-beginning 1) (match-beginning 4))
- (string-to-syntax "\"/")))
- (3 (if (or (match-beginning 1) (match-beginning 4))
- (string-to-syntax "\"/")
- (goto-char (match-end 2)))))
+ "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)")
+ (2 (string-to-syntax "\"/"))
+ (3 (string-to-syntax "\"/")))
("^=en\\(d\\)\\_>" (1 "!"))
("^\\(=\\)begin\\_>" (1 "!"))
;; Handle here documents.
diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
index 7a9d123..4f2e9e6 100644
--- a/test/indent/ruby.rb
+++ b/test/indent/ruby.rb
@@ -1,24 +1,25 @@
-# Don't mis-match "sub" at the end of words.
-a = asub / aslb + bsub / bslb;
-
+# Percent literals.
b = %Q{This is a "string"}
-c = %w(foo
+c = %w!foo
bar
- baz)
-d = %!hello!
+ baz!
+d = %(hello (nested) world)
# Don't propertize percent literals inside strings.
"(%s, %s)" % [123, 456]
-# Nor inside comments.
+# Or inside comments.
x = # "tot %q/to"; =
y = 2 / 3
-# 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
- toto = 1
- end
+# Regexp after whitelisted method.
+"abc".sub /b/, 'd'
+
+# Don't mis-match "sub" at the end of words.
+a = asub / aslb + bsub / bslb;
+
+# Highlight the regexp after "if".
+x = toto / foo if /do bar/ =~ "dobar"
# Some Cucumber code:
Given /toto/ do
--
1.7.10.msysgit.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
[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
1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2012-05-03 5:39 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
[-- Attachment #1: Type: text/plain, Size: 304 bytes --]
Hi again,
I missed the byte compilation error, arising from the fact that
`syntax-propertize-rules` evaluates regular expressions at compile time.
Options:
1) Remove customizability (see attached patch).
2) Regexp matches any method, then we check against the whitelist in a
new function.
-- Dmitry
[-- Attachment #2: 0003-lisp-progmodes-ruby-mode.el-Fix-byte-compilation-err.patch --]
[-- Type: text/plain, Size: 2302 bytes --]
From bb9ffe72bcb874aec06ca06b62576af8eca5f9f2 Mon Sep 17 00:00:00 2001
From: Dmitry Gutov <dgutov@yandex.ru>
Date: Thu, 3 May 2012 09:11:20 +0400
Subject: [PATCH 3/3] * lisp/progmodes/ruby-mode.el: Fix byte compilation
error
ruby-syntax-methods-before-regexp: make constant
ruby-syntax-methods-before-regexp, ruby-percent-literal-beg-re: wrap
together in (eval-and-compile)
---
lisp/progmodes/ruby-mode.el | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 14ce846..15c8246 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -226,13 +226,6 @@ Also ignores spaces after parenthesis when 'space."
"Use `ruby-encoding-map' to set encoding magic comment if this is non-nil."
:type 'boolean :group 'ruby)
-(defcustom 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."
- :group 'ruby)
-
;; Safe file variables
(put 'ruby-indent-tabs-mode 'safe-local-variable 'booleanp)
(put 'ruby-indent-level 'safe-local-variable 'integerp)
@@ -1124,9 +1117,16 @@ See `add-log-current-defun-function'."
(if (eval-when-compile (fboundp #'syntax-propertize-rules))
;; New code that works independently from font-lock.
(progn
- (defconst ruby-percent-literal-beg-re
- "\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
- "Regexp to match the beginning of percent literal.")
+ (eval-and-compile
+ (defconst ruby-percent-literal-beg-re
+ "\\(%\\)[qQrswWx]?\\([[: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."))
(defun ruby-syntax-propertize-function (start end)
"Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
--
1.7.10.msysgit.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-05-03 5:39 ` Dmitry Gutov
@ 2012-08-14 3:56 ` Dmitry Gutov
2012-08-14 12:40 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2012-08-14 3:56 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
[-- Attachment #1: Type: text/plain, Size: 284 bytes --]
Here are the 3 previously submitted patches as 2 files, slightly
reworded, and with proper ChangeLog entries.
I think the first one is quite important, and I've had a user emailing
me about it.
The second one deals with a fairly narrow edge case, so it's kinda optional.
--Dmitry
[-- Attachment #2: 01-6286-percent-literals.diff --]
[-- Type: text/plain, Size: 8832 bytes --]
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index cedf854..4f52796 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,15 @@
+2012-08-14 Dmitry Gutov <dgutov@yandex.ru>
+
+ * progmodes/ruby-mode.el (ruby-percent-literal-beg-re): New constant.
+ (ruby-syntax-general-delimiters-goto-beg): Rename to
+ `ruby-syntax-enclosing-percent-literal', improve literal type check.
+ (ruby-syntax-propertize-general-delimiters): Rename to
+ `ruby-syntax-propertize-percent-literal', it's a shorter and more
+ popular term. Adjust comments everywhere.
+ (ruby-syntax-propertize-percent-literal): Only propertize when not
+ inside a simple string or comment. When the literal is unclosed,
+ leave the text after it unpropertized.
+
2012-08-13 Andreas Schwab <schwab@linux-m68k.org>
* files.el (hack-local-variables-filter): Remove useless eval.
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 744dd43..42e1ac7 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -800,7 +800,7 @@ and `\\' when preceded by `?'."
;; (not (or (eolp) (looking-at "#")
;; (and (eq (car (nth 1 state)) ?{)
;; (looking-at "|"))))))
- ;; Not a regexp or general delimited literal.
+ ;; Not a regexp or percent literal.
(null (nth 0 (ruby-parse-region (or begin parse-start)
(point))))
(or (not (eq ?| (char-after (point))))
@@ -1169,17 +1169,22 @@ See `add-log-current-defun-function'."
(ruby-do-end-to-brace)))
(declare-function ruby-syntax-propertize-heredoc "ruby-mode" (limit))
-(declare-function ruby-syntax-general-delimiters-goto-beg "ruby-mode" ())
-(declare-function ruby-syntax-propertize-general-delimiters "ruby-mode" (limit))
+(declare-function ruby-syntax-enclosing-percent-literal "ruby-mode" (limit))
+(declare-function ruby-syntax-propertize-percent-literal "ruby-mode" (limit))
(if (eval-when-compile (fboundp #'syntax-propertize-rules))
;; New code that works independently from font-lock.
(progn
+ (eval-and-compile
+ (defconst ruby-percent-literal-beg-re
+ "\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
+ "Regexp to match the beginning of percent literal."))
+
(defun ruby-syntax-propertize-function (start end)
"Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
(goto-char start)
(ruby-syntax-propertize-heredoc end)
- (ruby-syntax-general-delimiters-goto-beg)
+ (ruby-syntax-enclosing-percent-literal end)
(funcall
(syntax-propertize-rules
;; #{ }, #$hoge, #@foo are not comments.
@@ -1222,8 +1227,8 @@ See `add-log-current-defun-function'."
'syntax-table (string-to-syntax "\""))
(ruby-syntax-propertize-heredoc end))))
;; Handle percent literals: %w(), %q{}, etc.
- ("\\(?:^\\|[[ \t\n<+(,=]\\)\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
- (1 (prog1 "|" (ruby-syntax-propertize-general-delimiters end)))))
+ ((concat "\\(?:^\\|[[ \t\n<+(,=]\\)" ruby-percent-literal-beg-re)
+ (1 (prog1 "|" (ruby-syntax-propertize-percent-literal end)))))
(point) end))
(defun ruby-syntax-propertize-heredoc (limit)
@@ -1251,40 +1256,46 @@ See `add-log-current-defun-function'."
;; inf-loop.
(if (< (point) start) (goto-char start))))))
- (defun ruby-syntax-general-delimiters-goto-beg ()
- (let ((state (syntax-ppss)))
- ;; Move to the start of the literal, in case it's multiline.
- ;; TODO: determine the literal type more reliably here?
+ (defun ruby-syntax-enclosing-percent-literal (limit)
+ (let ((state (syntax-ppss))
+ (start (point)))
+ ;; When already inside percent literal, re-propertize it.
(when (eq t (nth 3 state))
(goto-char (nth 8 state))
- (beginning-of-line))))
+ (when (looking-at ruby-percent-literal-beg-re)
+ (ruby-syntax-propertize-percent-literal limit))
+ (when (< (point) start) (goto-char start)))))
- (defun ruby-syntax-propertize-general-delimiters (limit)
+ (defun ruby-syntax-propertize-percent-literal (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 "|")))))
+ ;; Not inside a simple string or comment.
+ (when (eq t (nth 3 (syntax-ppss)))
+ (let* ((op (char-after))
+ (ops (char-to-string op))
+ (cl (or (cdr (aref (syntax-table) op))
+ (cdr (assoc op '((?< . ?>))))))
+ parse-sexp-lookup-properties)
+ (condition-case nil
+ (progn
+ (if cl ; 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))
+ ;; Found the closing delimiter.
+ (put-text-property (1- (point)) (point) 'syntax-table
+ (string-to-syntax "|")))
+ ;; Unclosed literal, leave the following text unpropertized.
+ ((scan-error search-failed) (goto-char limit))))))
)
;; For Emacsen where syntax-propertize-rules is not (yet) available,
@@ -1329,7 +1340,7 @@ This should only be called after matching against `ruby-here-doc-end-re'."
(4 (7 . ?/))
(6 (7 . ?/)))
("^=en\\(d\\)\\_>" 1 "!")
- ;; General delimited string.
+ ;; Percent literal.
("\\(^\\|[[ \t\n<+(,=]\\)\\(%[xrqQwW]?\\([^<[{(a-zA-Z0-9 \n]\\)[^\n\\\\]*\\(\\\\.[^\n\\\\]*\\)*\\(\\3\\)\\)"
(3 "\"")
(5 "\""))
diff --git a/test/ChangeLog b/test/ChangeLog
index a0163b5..9dbca3d 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,7 @@
+2012-08-14 Dmitry Gutov <dgutov@yandex.ru>
+
+ * indent/ruby.rb: New examples.
+
2012-08-12 Dmitry Gutov <dgutov@yandex.ru>
* automated/ruby-mode-tests.el (ruby-move-to-block-stops-at-opening)
diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
index c4a747a..7a9d123 100644
--- a/test/indent/ruby.rb
+++ b/test/indent/ruby.rb
@@ -7,6 +7,13 @@ c = %w(foo
baz)
d = %!hello!
+# Don't propertize percent literals inside strings.
+"(%s, %s)" % [123, 456]
+
+# Nor inside comments.
+x = # "tot %q/to"; =
+y = 2 / 3
+
# 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
[-- Attachment #3: 02-6286-regexps.diff --]
[-- Type: text/plain, Size: 5077 bytes --]
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 4f52796..f37c346 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -9,6 +9,9 @@
(ruby-syntax-propertize-percent-literal): Only propertize when not
inside a simple string or comment. When the literal is unclosed,
leave the text after it unpropertized.
+ (ruby-syntax-methods-before-regexp): New constant.
+ (ruby-syntax-propertize-function): Use it to recognize regexps.
+ Don't look at the text after regexp, just use the whitelist.
2012-08-13 Andreas Schwab <schwab@linux-m68k.org>
diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 42e1ac7..457c7fe 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -1178,7 +1178,13 @@ See `add-log-current-defun-function'."
(eval-and-compile
(defconst ruby-percent-literal-beg-re
"\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
- "Regexp to match the beginning of percent literal."))
+ "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."))
(defun ruby-syntax-propertize-function (start end)
"Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
@@ -1196,28 +1202,23 @@ See `add-log-current-defun-function'."
;; Not within a string.
(nth 3 (syntax-ppss (match-beginning 0))))
(string-to-syntax "\\"))))
- ;; Regexps: regexps are distinguished from division either because
- ;; of the keyword/symbol before them, or because of the code
- ;; following them.
+ ;; Regexps: regexps are distinguished from division because
+ ;; of the keyword, symbol, or method name before them.
((concat
;; Special tokens that can't be followed by a division operator.
- "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
+ "\\(^\\|[[=(,~?:;<>]"
+ ;; Control flow keywords and operators following bol or whitespace.
+ "\\|\\(?:^\\|\\s \\)"
(regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
- "or" "&&" "||"
- "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
- "\\)\\s *\\)?"
+ "or" "not" "&&" "||"))
+ ;; Method name from the list.
+ "\\|\\_<"
+ (regexp-opt ruby-syntax-methods-before-regexp)
+ "\\)\\s *"
;; The regular expression itself.
- "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
- ;; Special code that cannot follow a division operator.
- ;; FIXME: Just because the second slash of "/foo/ do bar" can't
- ;; be a division, doesn't mean it can't *start* a regexp, as in
- ;; "x = toto/foo; if /do bar/".
- "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
- (2 (when (or (match-beginning 1) (match-beginning 4))
- (string-to-syntax "\"/")))
- (3 (if (or (match-beginning 1) (match-beginning 4))
- (string-to-syntax "\"/")
- (goto-char (match-end 2)))))
+ "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)")
+ (2 (string-to-syntax "\"/"))
+ (3 (string-to-syntax "\"/")))
("^=en\\(d\\)\\_>" (1 "!"))
("^\\(=\\)begin\\_>" (1 "!"))
;; Handle here documents.
diff --git a/test/ChangeLog b/test/ChangeLog
index 9dbca3d..f1bf458 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,6 +1,6 @@
2012-08-14 Dmitry Gutov <dgutov@yandex.ru>
- * indent/ruby.rb: New examples.
+ * indent/ruby.rb: Rearrange examples, add new ones.
2012-08-12 Dmitry Gutov <dgutov@yandex.ru>
diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
index 7a9d123..4f2e9e6 100644
--- a/test/indent/ruby.rb
+++ b/test/indent/ruby.rb
@@ -1,24 +1,25 @@
-# Don't mis-match "sub" at the end of words.
-a = asub / aslb + bsub / bslb;
-
+# Percent literals.
b = %Q{This is a "string"}
-c = %w(foo
+c = %w!foo
bar
- baz)
-d = %!hello!
+ baz!
+d = %(hello (nested) world)
# Don't propertize percent literals inside strings.
"(%s, %s)" % [123, 456]
-# Nor inside comments.
+# Or inside comments.
x = # "tot %q/to"; =
y = 2 / 3
-# 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
- toto = 1
- end
+# Regexp after whitelisted method.
+"abc".sub /b/, 'd'
+
+# Don't mis-match "sub" at the end of words.
+a = asub / aslb + bsub / bslb;
+
+# Highlight the regexp after "if".
+x = toto / foo if /do bar/ =~ "dobar"
# Some Cucumber code:
Given /toto/ do
^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-08-14 3:56 ` Dmitry Gutov
@ 2012-08-14 12:40 ` Stefan Monnier
2012-08-14 17:46 ` Dmitry Gutov
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2012-08-14 12:40 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 6286
> I think the first one is quite important, and I've had a user emailing me
> about it.
Thanks, installed.
> The second one deals with a fairly narrow edge case, so it's kinda optional.
Thanks, installed.
I kind of remember something about this second case, and remember being
bothered by the fact that ruby-syntax-methods-before-regexp would need
to be adjusted by the user since it can depend on its locale
(especially the Given/Then/When).
I also vaguely remember that this problem was related to the reason why
I added the "look after the regexp" test.
It would probably be better if you could commit those changes yourself.
If you want to do that, please request membership in the "emacs" group
from your savannah account (which you may have to create beforehand).
Stefan
> diff --git a/lisp/ChangeLog b/lisp/ChangeLog
> index 4f52796..f37c346 100644
> --- a/lisp/ChangeLog
> +++ b/lisp/ChangeLog
> @@ -9,6 +9,9 @@
> (ruby-syntax-propertize-percent-literal): Only propertize when not
> inside a simple string or comment. When the literal is unclosed,
> leave the text after it unpropertized.
> + (ruby-syntax-methods-before-regexp): New constant.
> + (ruby-syntax-propertize-function): Use it to recognize regexps.
> + Don't look at the text after regexp, just use the whitelist.
> 2012-08-13 Andreas Schwab <schwab@linux-m68k.org>
> diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
> index 42e1ac7..457c7fe 100644
> --- a/lisp/progmodes/ruby-mode.el
> +++ b/lisp/progmodes/ruby-mode.el
> @@ -1178,7 +1178,13 @@ See `add-log-current-defun-function'."
> (eval-and-compile
> (defconst ruby-percent-literal-beg-re
> "\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
> - "Regexp to match the beginning of percent literal."))
> + "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."))
> (defun ruby-syntax-propertize-function (start end)
> "Syntactic keywords for Ruby mode. See `syntax-propertize-function'."
> @@ -1196,28 +1202,23 @@ See `add-log-current-defun-function'."
> ;; Not within a string.
> (nth 3 (syntax-ppss (match-beginning 0))))
> (string-to-syntax "\\"))))
> - ;; Regexps: regexps are distinguished from division either because
> - ;; of the keyword/symbol before them, or because of the code
> - ;; following them.
> + ;; Regexps: regexps are distinguished from division because
> + ;; of the keyword, symbol, or method name before them.
> ((concat
> ;; Special tokens that can't be followed by a division operator.
> - "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
> + "\\(^\\|[[=(,~?:;<>]"
> + ;; Control flow keywords and operators following bol or whitespace.
> + "\\|\\(?:^\\|\\s \\)"
> (regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
> - "or" "&&" "||"
> - "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
> - "\\)\\s *\\)?"
> + "or" "not" "&&" "||"))
> + ;; Method name from the list.
> + "\\|\\_<"
> + (regexp-opt ruby-syntax-methods-before-regexp)
> + "\\)\\s *"
> ;; The regular expression itself.
> - "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
> - ;; Special code that cannot follow a division operator.
> - ;; FIXME: Just because the second slash of "/foo/ do bar" can't
> - ;; be a division, doesn't mean it can't *start* a regexp, as in
> - ;; "x = toto/foo; if /do bar/".
> - "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
> - (2 (when (or (match-beginning 1) (match-beginning 4))
> - (string-to-syntax "\"/")))
> - (3 (if (or (match-beginning 1) (match-beginning 4))
> - (string-to-syntax "\"/")
> - (goto-char (match-end 2)))))
> + "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)")
> + (2 (string-to-syntax "\"/"))
> + (3 (string-to-syntax "\"/")))
> ("^=en\\(d\\)\\_>" (1 "!"))
> ("^\\(=\\)begin\\_>" (1 "!"))
> ;; Handle here documents.
> diff --git a/test/ChangeLog b/test/ChangeLog
> index 9dbca3d..f1bf458 100644
> --- a/test/ChangeLog
> +++ b/test/ChangeLog
> @@ -1,6 +1,6 @@
> 2012-08-14 Dmitry Gutov <dgutov@yandex.ru>
> - * indent/ruby.rb: New examples.
> + * indent/ruby.rb: Rearrange examples, add new ones.
> 2012-08-12 Dmitry Gutov <dgutov@yandex.ru>
> diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
> index 7a9d123..4f2e9e6 100644
> --- a/test/indent/ruby.rb
> +++ b/test/indent/ruby.rb
> @@ -1,24 +1,25 @@
> -# Don't mis-match "sub" at the end of words.
> -a = asub / aslb + bsub / bslb;
> -
> +# Percent literals.
> b = %Q{This is a "string"}
> -c = %w(foo
> +c = %w!foo
> bar
> - baz)
> -d = %!hello!
> + baz!
> +d = %(hello (nested) world)
> # Don't propertize percent literals inside strings.
> "(%s, %s)" % [123, 456]
> -# Nor inside comments.
> +# Or inside comments.
> x = # "tot %q/to"; =
> y = 2 / 3
> -# 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
> - toto = 1
> - end
> +# Regexp after whitelisted method.
> +"abc".sub /b/, 'd'
> +
> +# Don't mis-match "sub" at the end of words.
> +a = asub / aslb + bsub / bslb;
> +
> +# Highlight the regexp after "if".
> +x = toto / foo if /do bar/ =~ "dobar"
> # Some Cucumber code:
> Given /toto/ do
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-08-14 12:40 ` Stefan Monnier
@ 2012-08-14 17:46 ` Dmitry Gutov
2012-08-15 2:33 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2012-08-14 17:46 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 6286
On 14.08.2012 16:40, Stefan Monnier wrote:
> I kind of remember something about this second case, and remember being
> bothered by the fact that ruby-syntax-methods-before-regexp would need
> to be adjusted by the user since it can depend on its locale
> (especially the Given/Then/When).
While Cucumber features can be localized, ruby-mode should only be used
with step definitions [1], and those just contain Ruby code, so the
Given/Then/When method names would be the same, AFAIK.
A user might want to be able to add a new method to the list,
theoretically, but paren-less calling convention is mostly used in
DSL-like code (that means, with methods provided by some framework or
other), so I say we wait until such feature request actually comes.
> I also vaguely remember that this problem was related to the reason why
> I added the "look after the regexp" test.
That was in the original patch I submitted, mostly because it looked
like a clever solution. But the conservative approach should work, too:
the canonical (I think) TextMate bundle uses the whitelist [2] without
major complaints.
> It would probably be better if you could commit those changes yourself.
> If you want to do that, please request membership in the "emacs" group
> from your savannah account (which you may have to create beforehand).
Requested, thanks.
I think we're finished with this bug, though, unless you'd like to see
some other changes.
--Dmitry
[1] https://github.com/cucumber/cucumber/wiki/Step-Definitions
[2]
https://github.com/drnic/ruby-tmbundle/blob/master/Syntaxes/Ruby.plist#L882
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#6286: General delimited literals in ruby-mode patch
2012-08-14 17:46 ` Dmitry Gutov
@ 2012-08-15 2:33 ` Stefan Monnier
0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2012-08-15 2:33 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 6286-done
> I think we're finished with this bug, though, unless you'd like to see some
> other changes.
Indeed, thanks,
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-08-15 2:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <8739ammd8l.fsf@yandex.ru>
[not found] ` <87k43vecyt.fsf@yandex.ru>
2012-04-24 15:43 ` bug#6286: General delimited literals in ruby-mode patch 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
[not found] ` <87ehu3mga2.fsf@yandex.ru>
2012-04-24 17:09 ` Stefan Monnier
2012-04-25 3:03 ` Dmitry Gutov
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).