bug#60186: 29.0.60; ruby-mode indentation of multi-line expressions

[Dmitry Gutov]

On 24/12/2022 02:17, Aaron Jensen wrote:
> On Fri, Dec 23, 2022 at 5:26 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> > Is that also true for the other "codebases you've seen" referred to in
> > the first message here?
>
> Mostly we work with Rails (and I try to avoid looking at that code as
> much as I can, though I often find myself in there...) and the
> Eventide framework: https://github.com/eventide-project

Thanks. It's very neat. I couldn't find any big app in there, though. 
And that's where the things tend to become more hairy.

> Given that the two founders of that are on my team we tend to follow
> their indentation and coding style (and it is well-thought out, with
> everything considered with respect to human cognition/eye tracking
> studies/etc.). That's probably the best example I could offer, though
> what you will find in there is that not many lines of code span more
> than one line. They (and we) tend to allow longer lines when the right
> side of the line is less important.

With code rarely spanning multiple lines, the current indentation logic 
of ruby-mode is probably working fine too.

> > > With either indentation style, the
> > > first argument (which is the most significant one when a method is
> > > properly designed) will have the least presence when scanning. It's
> > > just not a good format in my experience. In our code we take it a step
> > > further and always use parentheses except for in class level "macros".
> >
> > That's also my preference and true of the code I've seen. But "class
> > level macros" are often an important part of projects, be that
> > ActiveRecord validations, or DSL-like calls in Grape (routes, params, etc).
> >
> > So I wonder whether we should alter parenless calls' indentation for the
> > "simplified" style.
>
> I think I would tend towards saying yes, make it simple/like the chef
> codebase. That's consistent and if one wants to line things up, then
> one can use parens, even with class level macros (or use long lines
> and not care).

All right. In the latest iteration of the patch (attached) I've split 
off the block's indentation behavior into a separate option and altered 
the indentation of the parenless calls.

In the more complex cases the results are definitely interesting:

  method arg1,
    method2 arg2,
    arg3

  zzz = method (a + b),
    c, :d => :e,
    f: g

> > Though they also like to line up the keyword arguments according to
> > Rubocop's defaults
> > (https://github.com/spree/spree/blob/main/core/app/models/spree/product.rb#L63),
> > something we don't support yet.
>
> This line is rather painful to read and inconsistent with the call
> just below it on line 70. I would certainly not advocate for that.

I think it's consistent in the sense that the "keyword" section of the 
arguments is vertically aligned in both cases. It's a popular Rubocop 
rule, apparently.

Whether it's useful, though, I'm doubtful too.

> > Do you have a source-available example of a project in your preferred
> > coding style?
> >
> > Chef, perhaps?
> >
> > https://github.com/chef/chef/blob/main/lib/chef/application/base.rb
> > https://github.com/chef/chef/blob/main/lib/chef/application/client.rb
>
> Yeah, I think this is probably the closest with regard to indentation
> aside from Eventide, but again, you won't find much indentation there.
> Whatever you find will likely be Vim's default, which I believe is the
> same simple two space indent we are discussing.

Okay.

> > > This means that any time we decide to split a method invocation on
> > > multiple lines we use the basic newline after ( style.
> >
> > For "class-level macros" as well?
>
> I found inconsistencies in our codebase, so it is time for us to
> establish a new norm. My guess is that we will land on using parens
> for it. When we do it without parens, it is in the simplified style.
>
> Thank you for your research and work on this, I appreciate it.

Thank you too.

We could also discuss cases like

  foo = bar({
              tee: 1,
              qux: 2
           })

  baz([
        1,
        2,
        3
      ])

but those would be an orthogonal feature. And I don't see them much in 
the wild, for some reason.

ruby-simplified-indent-v5.diff
Description: Text Data