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

[Aaron Jensen]

On Sat, Dec 24, 2022 at 5:47 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> 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.

Funny thing, that. That's intentional. Our client's codebase we built
doesn't have any big apps either. Different subject though.


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

We see more in our application (and that's closed source, so I cannot
share it). Suffice it to say, it came up enough that all the Emacs
users on our team have had trouble with it, so this work will be
appreciated.

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

I don't know what I'd expect here, other than to have a conversation
with the dev who wrote it to explain why we use parentheses :) In
other words, I'm fine with this indentation and I couldn't even tell
you the precedence here off the top of my head.

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

Yeah, this looks right to me too.


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

The same logic would apply. It doesn't matter how many indent starters
there are in a line, the indentation should only increase by one:


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

baz([
  1,
  2,
  3
])

Of course, that begs the question what happens if you do this:

baz([
  1,
  2,
  3
]
)

And, I think again, the answer is a social one, rather than a technical one.

enh-ruby-mode and vim both do this this:

baz([
  1,
  2,
  3
]
   )

I would not even attempt to explain why in this situation they rely on
lining up parens, but this is what they do. So, we could either
emulate this, or chart our own course. I would have expected it to
look like the example I gave with the ] and ) both at the beginning of
the line with no indentation. The Vim way looks worse though, which in
this instance may be a feature :)

Aaron