[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6286: General delimited literals in ruby-mode patch
From: |
Stefan Monnier |
Subject: |
bug#6286: General delimited literals in ruby-mode patch |
Date: |
Wed, 25 Apr 2012 10:15:21 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.94 (gnu/linux) |
> 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