bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Dmitry Gutov]

Hi Stefan,

thanks for starting on this feature. Comments and questions follow.

> revno: 114639
> revision-id: monnier@iro.umontreal.ca-20131012204050-2kntbtokz1wa3mk5
> parent: eggert@cs.ucla.edu-20131012200038-eeyl9fi1y3vr0vwy
> committer: Stefan Monnier <monnier@iro.umontreal.ca>
> branch nick: trunk
> timestamp: Sat 2013-10-12 16:40:50 -0400

> ...

> +(defun ruby-smie--args-separator-p (pos)
> +  (and
> +   (eq ?w (char-syntax (char-before)))
> +   (< pos (point-max))
> +   (memq (char-syntax (char-after pos)) '(?w ?\"))))

I've made a change to the first condition in 114655, but the last line
is still very inadequate.

The first argument can be any kind of literal, parenthesized expression,
or even (if we want to get fancy) something like `begin foo; bar end'.
(See new examples is indent/ruby.rb).

So basically, I think we'd like to check if the token following POS is
either not "special", or if it is, it begins an expression. Can we do
that?

> === modified file 'test/indent/ruby.rb'
> --- a/test/indent/ruby.rb	2013-10-11 20:45:14 +0000
> +++ b/test/indent/ruby.rb	2013-10-12 20:40:50 +0000
> @@ -170,3 +170,7 @@
>  if foo &&
>      bar
>  end
> +
> +method1 arg1,                   # bug#15594
> +        method2 arg2,
> +                arg3

Please note that you've added the example to the "currently failing"
part of the file. I've moved it now.

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Stefan Monnier]

> So basically, I think we'd like to check if the token following POS is
> either not "special", or if it is, it begins an expression.  Can we do
> that?

I don't see anything that would stop us.  But it'll be more difficult to handle

    method(arg1),
          arg2
or
    method{arg1},
          arg2

since there's no space to use as "implicit method call infix operator".
          

        Stefan


=== modified file 'test/indent/ruby.rb'
--- test/indent/ruby.rb	2013-10-14 01:51:20 +0000
+++ test/indent/ruby.rb	2013-10-14 13:39:08 +0000
@@ -177,10 +177,12 @@
 foo_bar_tee(1, 2, 3)
   .qux
 
+# Shouldn't "bar" be aligned with "foo"?  --Stef
 if foo &&
     bar
 end
 
+# Shouldn't "arg2" be aligned with "!" rather than with "arg1"?  --Stef
 method !arg1,
         arg2
 

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Dmitry Gutov]

On 14.10.2013 16:44, Stefan Monnier wrote:
>> So basically, I think we'd like to check if the token following POS is
>> either not "special", or if it is, it begins an expression.  Can we do
>> that?
>
> ...(+)  But it'll be more difficult to handle
>...(++)
> since there's no space to use as "implicit method call infix operator".

No, space is significant(++), I'm not suggesting to revert the addition 
of the " @ " token. But we need a better predicate in 
`ruby-smie--args-separator-p' to check that the thing after POS is 
probably an argument. "Starts with a word character" is too narrow.

 > (+) I don't see anything that would stop us.

How would that look?

(unless (member (save-excursion (ruby-smie--forward-token) '("]" "}" 
"end" "+" "-" "?" ":" ...)))

?

Can we extract the "prohibited" list from the already defined grammar?

Or should the check be more like "is the next token in 
`ruby-smie-grammar', and if yes, is its left priority more than ' @ 's 
right priority"?

(++)
 >      method(arg1),
 >            arg2
 > or
 >      method{arg1},
 >            arg2
 >

Both examples would result in syntax errors. The second one doubly so, 
since the first arg there is not a hash, but a block, and a block can 
only come after all other arguments.

Even if the first argument were {:a => 1, :b => 2}, actually, it won't 
work because "{" in this position is parsed as the beginning of a block 
(that means one of the examples I added is wrong, sorry!).

You've probably already found this, but on the off chance you haven't, 
here's its syntax in (incomplete, somewhat outdated, etc) BNF form: 
http://www.cse.buffalo.edu/~regan/cse305/RubyBNF.pdf

> +# Shouldn't "bar" be aligned with "foo"?  --Stef
>   if foo &&
>       bar
>   end

Maybe. Either option would be better than the current behavior. I've 
picked this one because the old engine indents it like so, and 
additional indentation of 2 (compared to the if body) would likely be 
more useful that 1, when reading the code.


> +# Shouldn't "arg2" be aligned with "!" rather than with "arg1"?  --Stef
>   method !arg1,
>           arg2

Yes, sorry. Fixed, and added a couple of new examples, like usual.

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Stefan Monnier]

> No, space is significant(++),

I guess we're lucky.

> I'm not suggesting to revert the addition of
> the " @ " token. But we need a better predicate in
> ruby-smie--args-separator-p' to check that the thing after POS is probably
> an argument. "Starts with a word character" is too narrow.

> How would that look?
> (unless (member (save-excursion (ruby-smie--forward-token) '("]" "}" "end"
> "+" "-" "?" ":" ...)))

(looking-at "\\s)\\|\\s.") ?

> Or should the check be more like "is the next token in `ruby-smie-grammar',
> and if yes, is its left priority more than ' @ 's right priority"?

Calling ruby-smie--forward-token is a bit dangerous since that function
might itself be called from ruby-smie--forward-token.  It might work,
but you'll have to think hard about why an inf-loop is not possible.

> You've probably already found this, but on the off chance you haven't,
> here's its syntax in (incomplete, somewhat outdated, etc) BNF form:
> http://www.cse.buffalo.edu/~regan/cse305/RubyBNF.pdf

Please add this URL in a comment somewhere near ruby-smie-grammar (for
example).

>> +# Shouldn't "bar" be aligned with "foo"?  --Stef
>> if foo &&
>> bar
>> end

> Maybe. Either option would be better than the current behavior. I've picked
> this one because the old engine indents it like so, and additional
> indentation of 2 (compared to the if body) would likely be more useful that
> 1, when reading the code.

Getting `foo' and `bar' aligned is just a matter of adding && to the set
of infix operators (i.e. completing the table of infix operators).
Getting `bar' to be indented one more than `foo' here but not in other
cases of "foo && \n bar" would require more work.


        Stefan

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Dmitry Gutov]

On 15.10.2013 07:31, Stefan Monnier wrote:
>> How would that look?
>> (unless (member (save-excursion (ruby-smie--forward-token) '("]" "}" "end"
>> "+" "-" "?" ":" ...)))
>
> (looking-at "\\s)\\|\\s.") ?

I guess this is better, but it has both false negatives (unary operators 
like -, ~ and !) and false positives (all non-opener keywords).

>> Or should the check be more like "is the next token in `ruby-smie-grammar',
>> and if yes, is its left priority more than ' @ 's right priority"?
>
> Calling ruby-smie--forward-token is a bit dangerous since that function
> might itself be called from ruby-smie--forward-token.  It might work,
> but you'll have to think hard about why an inf-loop is not possible.

Hopefully because both `ruby-smie--forward-token' and 
`ruby-smie--backward-token' would only call `ruby-smie--forward-token', 
and only when (> pos (point)), IOW there has to be some whitespace 
skipping done between the recursive calls.

>> You've probably already found this, but on the off chance you haven't,
>> here's its syntax in (incomplete, somewhat outdated, etc) BNF form:
>> http://www.cse.buffalo.edu/~regan/cse305/RubyBNF.pdf
>
> Please add this URL in a comment somewhere near ruby-smie-grammar (for
> example).

Done.

> Getting `foo' and `bar' aligned is just a matter of adding && to the set
> of infix operators (i.e. completing the table of infix operators).
> Getting `bar' to be indented one more than `foo' here but not in other
> cases of "foo && \n bar" would require more work.

Ok, let's go with the former for now.

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Stefan Monnier]

>> Calling ruby-smie--forward-token is a bit dangerous since that function
>> might itself be called from ruby-smie--forward-token.  It might work,
>> but you'll have to think hard about why an inf-loop is not possible.
> Hopefully because both `ruby-smie--forward-token' and
> ruby-smie--backward-token' would only call `ruby-smie--forward-token', and
> only when (> pos (point)), IOW there has to be some whitespace skipping done
> between the recursive calls.

Only recursing in one direction (only forward or only backward) is
a good way to guarantee progress, indeed.  But currently
ruby-smie--implicit-semi-p calls ruby-smie--backward-token, so beware.


        Stefan

bug#15594: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar): Add rule for paren-free

[Dmitry Gutov]

On 21.10.2013 17:25, Stefan Monnier wrote:
>>> Calling ruby-smie--forward-token is a bit dangerous since that function
>>> might itself be called from ruby-smie--forward-token.  It might work,
>>> but you'll have to think hard about why an inf-loop is not possible.
>> Hopefully because both `ruby-smie--forward-token' and
>> ruby-smie--backward-token' would only call `ruby-smie--forward-token', and
>> only when (> pos (point)), IOW there has to be some whitespace skipping done
>> between the recursive calls.
>
> Only recursing in one direction (only forward or only backward) is
> a good way to guarantee progress, indeed.  But currently
> ruby-smie--implicit-semi-p calls ruby-smie--backward-token, so beware.

That's a good point. And this would probably be more expensive approach, 
so I went with local search, for now.