GNU bug report logs - #15594
24.3; Indentation of method arguments without parentheses in ruby-mode is broken

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15594 in the body.
You can then email your comments to 15594 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Bozhidar Batsov <bozhidar.batsov <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3; Indentation of method arguments without parentheses in
 ruby-mode is broken
Date: Sat, 12 Oct 2013 09:24:49 +0300

[Message part 1 (text/plain, inline)]

In Ruby one can invoke methods with or without parentheses:

some_method(arg1, arg2)
some_method arg1, arg2

ruby-mode currently does not handle properly the indentation of calls without parentheses that span several lines:

# indented properly in 24.3
some_method(arg1,
                     arg2)

# not indented at all in 24.3
some_method arg1,
arg2

Ideally the second code example would be indented by ruby-mode like this:

some_method arg1,
                     arg2

-- 
Cheers,
Bozhidar

[Message part 2 (text/html, inline)]

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Bozhidar Batsov <bozhidar.batsov <at> gmail.com>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: bug#15594: 24.3;
 Indentation of method arguments without parentheses in ruby-mode is
 broken
Date: Sat, 12 Oct 2013 10:37:17 -0400

> In Ruby one can invoke methods with or without parentheses:
> some_method(arg1, arg2)
> some_method arg1, arg2

Interesting.  How is the following parsed, then:

   some_method arg1, other_method arg2, arg3, arg4


-- Stefan

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

From: Bozhidar Batsov <bozhidar.batsov <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: bug#15594: 24.3; Indentation of method arguments without
 parentheses in ruby-mode is broken
Date: Sat, 12 Oct 2013 17:44:11 +0300

[Message part 1 (text/plain, inline)]

On Saturday, October 12, 2013 at 5:37 PM, Stefan Monnier wrote:
> > In Ruby one can invoke methods with or without parentheses:
> > some_method(arg1, arg2)
> > some_method arg1, arg2
> > 
> 
> 
> Interesting. How is the following parsed, then:
> 
> some_method arg1, other_method arg2, arg3, arg4
There are some limitations, of course. Ambiguous cases generally result in parsing errors:

Parser::CurrentRuby.parse('some_method arg1, other_method arg2, arg3, arg4')
(string):1:32: error: unexpected token tIDENTIFIER
some_method arg1, other_method arg2, arg3, arg4
                                                  ^^^^
Parser::SyntaxError: unexpected token tIDENTIFIER 

If we remove arg1, however, there is no ambiguity:

[21] pry(main)> Parser::CurrentRuby.parse('some_method other_method arg2, arg3, arg4')
=> (send nil :some_method
  (send nil :other_method
    (send nil :arg2)
    (send nil :arg3)
    (send nil :arg4)))

> 
> 
> -- Stefan 

[Message part 2 (text/html, inline)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15594 <at> debbugs.gnu.org, emacs-devel <at> gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Mon, 14 Oct 2013 05:06:38 +0300

Hi Stefan,

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

> revno: 114639
> revision-id: monnier <at> iro.umontreal.ca-20131012204050-2kntbtokz1wa3mk5
> parent: eggert <at> cs.ucla.edu-20131012200038-eeyl9fi1y3vr0vwy
> committer: Stefan Monnier <monnier <at> 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.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 15594 <at> debbugs.gnu.org, emacs-devel <at> gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Mon, 14 Oct 2013 09:44:06 -0400

> 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
 

Message #22 received at 15594-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Bozhidar Batsov <bozhidar.batsov <at> gmail.com>
Cc: 15594-done <at> debbugs.gnu.org
Subject: Re: bug#15594: 24.3;
 Indentation of method arguments without parentheses in ruby-mode is
 broken
Date: Sat, 12 Oct 2013 16:41:26 -0400

>> > In Ruby one can invoke methods with or without parentheses:
>> > some_method(arg1, arg2)
>> > some_method arg1, arg2

I just added some support for it, thank you,


        Stefan

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Tue, 15 Oct 2013 04:23:50 +0300

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.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Mon, 14 Oct 2013 23:31:15 -0400

> 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

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Mon, 21 Oct 2013 10:02:38 +0400

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.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Mon, 21 Oct 2013 09:25:07 -0400

>> 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

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15594 <at> debbugs.gnu.org
Subject: Re: trunk r114639: * lisp/progmodes/ruby-mode.el (ruby-smie-grammar):
 Add rule for paren-free
Date: Sat, 26 Oct 2013 05:19:03 +0400

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.

Previous Next

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