Subject: bug#6286: General delimited literals in ruby-mode patch

From: Stefan Monnier <[email-masked]>
Date: Wed, 25 Apr 2012 10:15:21 -0400

> 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