GNU bug report logs - #6286
Ruby Mode Missing Syntax

Previous Next

Package: emacs;

Reported by: Nick Ewing <nick <at> nickewing.net>

Date: Thu, 27 May 2010 22:02:02 UTC

Severity: normal

Tags: patch

Done: Stefan Monnier <monnier <at> iro.umontreal.ca>

Bug is archived. No further changes may be made.

Forwarded to http://lists.gnu.org/archive/html/emacs-devel/2012-02/msg00247.html

Full log


Message #24 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
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




This bug report was last modified 12 years and 336 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.