bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

Artur Malabarba-2
1. (setq ruby-align-chained-calls t)
2. (setq ruby-use-smie t)
3. Open a file in ruby-mode, insert the following and indent it

----------
some_variable.where.not(x: nil)
             .where(y: 2)
----------

Expected behaviour: Nothing would happen, the code is already properly
indented.

What actually happens: The code gets indented as follows

----------
some_variable.where.not(x: nil)
                   .where(y: 2)
----------

Note that this is conflicts with the indentation enforced by rubocop.
Artur
Reply | Threaded
Open this post in threaded view
|

bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

Dmitry Gutov
On 8/22/18 2:36 PM, Artur Malabarba wrote:

> 1. (setq ruby-align-chained-calls t)
> 2. (setq ruby-use-smie t)
> 3. Open a file in ruby-mode, insert the following and indent it
>
> ----------
> some_variable.where.not(x: nil)
>               .where(y: 2)
> ----------
>
> Expected behaviour: Nothing would happen, the code is already properly
> indented.
>
> What actually happens: The code gets indented as follows
>
> ----------
> some_variable.where.not(x: nil)
>                     .where(y: 2)
> ----------
>
> Note that this is conflicts with the indentation enforced by rubocop.

I'd like to point out that this is exactly the behavior Bozhidar asked
for, back when this variable was introduced. See:

http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html

and in particular the Example 1 in the referenced comment:

https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622

So we even have a test (ruby-align-chained-calls) that check that the
alignment is do to the last dot, and not to the first one.



Reply | Threaded
Open this post in threaded view
|

bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

Artur Malabarba-2
IIUC, bozhidar was requesting that the dots be aligned to the dot above (as opposed to being indented by only 2 spaces). He didn't say what should happen if one of the lines has multiple dots in it.

The linked github comment does explicitly request the aligning to the last dot, but it's the only comment that requests that on a very long discussion that was largely focused on a different topic (whether or not to use trailing dots).

Bozhidar, do you have an opinion on this?

Artur


On Wed, 22 Aug 2018 at 09:50, Dmitry Gutov <[hidden email]> wrote:
On 8/22/18 2:36 PM, Artur Malabarba wrote:
> 1. (setq ruby-align-chained-calls t)
> 2. (setq ruby-use-smie t)
> 3. Open a file in ruby-mode, insert the following and indent it
>
> ----------
> some_variable.where.not(x: nil)
>               .where(y: 2)
> ----------
>
> Expected behaviour: Nothing would happen, the code is already properly
> indented.
>
> What actually happens: The code gets indented as follows
>
> ----------
> some_variable.where.not(x: nil)
>                     .where(y: 2)
> ----------
>
> Note that this is conflicts with the indentation enforced by rubocop.

I'd like to point out that this is exactly the behavior Bozhidar asked
for, back when this variable was introduced. See:

http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html

and in particular the Example 1 in the referenced comment:

https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622

So we even have a test (ruby-align-chained-calls) that check that the
alignment is do to the last dot, and not to the first one.
Reply | Threaded
Open this post in threaded view
|

bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

Bozhidar Batsov
Sorry for the radio silence - I've been super busy lately. 

It's hard for me to understand the indentation in the examples in the email (as it seems the same to me). Very simply put - the idea is to align multi-line chained calls on the `.`, as opposed to just nest them under the root receiver as we'd normally do.

I think Dmitry implemented this great and it's behaving just as it's supposed to be behaving. Perhaps you misunderstood how this was supposed to behave? What's the indentation you expected? 

On Sun, 28 Oct 2018 at 00:22, Artur Malabarba <[hidden email]> wrote:
IIUC, bozhidar was requesting that the dots be aligned to the dot above (as opposed to being indented by only 2 spaces). He didn't say what should happen if one of the lines has multiple dots in it.

The linked github comment does explicitly request the aligning to the last dot, but it's the only comment that requests that on a very long discussion that was largely focused on a different topic (whether or not to use trailing dots).

Bozhidar, do you have an opinion on this?

Artur


On Wed, 22 Aug 2018 at 09:50, Dmitry Gutov <[hidden email]> wrote:
On 8/22/18 2:36 PM, Artur Malabarba wrote:
> 1. (setq ruby-align-chained-calls t)
> 2. (setq ruby-use-smie t)
> 3. Open a file in ruby-mode, insert the following and indent it
>
> ----------
> some_variable.where.not(x: nil)
>               .where(y: 2)
> ----------
>
> Expected behaviour: Nothing would happen, the code is already properly
> indented.
>
> What actually happens: The code gets indented as follows
>
> ----------
> some_variable.where.not(x: nil)
>                     .where(y: 2)
> ----------
>
> Note that this is conflicts with the indentation enforced by rubocop.

I'd like to point out that this is exactly the behavior Bozhidar asked
for, back when this variable was introduced. See:

http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html

and in particular the Example 1 in the referenced comment:

https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622

So we even have a test (ruby-align-chained-calls) that check that the
alignment is do to the last dot, and not to the first one.
Reply | Threaded
Open this post in threaded view
|

bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t

Wendel Scardua
In reply to this post by Artur Malabarba-2
I was about to open a bug about this same issue. Seeing this thread, there seems to have been some misunderstanding between the parts.

Bhozidar thought it was about indenting on the dot vs indenting according to the previous line, and in that aspect the feature is working correctly.

But when there are multiple dots on a line, their style guide enforces indenting on the *first* dot, not the last, while (setq ruby-align-chained-calls t) - supposedly based on it - enforces the latter.