Subject: Re: bug#60321: 29.0.60; ruby-mode indentation of hash or array as first arg in multiline method call

From: Aaron Jensen <[email-masked]>
Date: Sun, 1 Sep 2024 20:49:58 -0400

On Sun, Sep 1, 2024 at 8:19 PM Dmitry Gutov <[email-masked]> wrote:
>
> On 01/09/2024 22:28, Aaron Jensen wrote:
>
> > Here's a corrected patch for that particular example. Thank you for
> > finding that. I think I missed it because as long as you type the code
> > in, it indents fine. I still have a lot to understand about SMIE, so if
> > anything looks off in my patch, please let me know.
>
> Thanks! Just being thorough. We can add this example to the args-indent
> test file, too.
>
> Here's a bonus example which looks off but would be more difficult to
> fix (and it's not urgent, given the expression is in mixed styles):
>
> foo([{
>    a: 2
> },
>       {
>         b: 3
>       },
>       4
>      ])

Yes, that's connected to the case I mentioned... how do you think it
should be indented? I wonder if it should just be 2 spaces in (rather
than aligned with the opening bracket)

> It would be nice to at least handle the last arg correctly - maybe we'll
> just get that supported in the ts mode at some later date.
>
> > I didn't change the default. I wasn't sure if you wanted to change the
> > defaults of all of the variables you added in the last round or just
> > this one, so I'll let you handle that the way you want to.
>
> Sure. I think we can add this into Emacs 30 too, while the change is off
> by default.

Sounds good.

> A few other things:
>
> * I think the docstring should say "Set it to nil to align to the line
> with the open bracket" - it doesn't necessarily align to the beginning
> of the statement (seems like a good thing).

Good call.

> * Let's change the first example to this, for less ambiguity?
>
>    foo
>      .update({
>        key => value,
>        other_key:
>      }, {
>        key => value,
>        other_key:
>      })
>

Done

> * Support for the new option in ruby-ts-mode (it's good to have parity).
> Could you take the attached patch for a spin? Seems to work here, but
> I'd like to have an extra confirmation.

I don't have the treesitter stuff installed at the moment, will try
this out shortly.

Thanks,

Aaron