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

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

[Aaron Jensen]

Follow-up to bug#60110

I prefer rather simplictic indentation for Ruby (and this appears to be
pretty common from codebases I've seen). Essentially, the rule is: If an
expression continues on another line, indent it once. 

Current:

some_variable = some_object.
                  some_method

Desired:

some_variable = some_object.
  some_method

Current:

some_variable = some_number + some_other_number *
                              some_third_number + some_fourth_number -
                some_fifth_number

Desired:

some_variable = some_number + some_other_number *
  some_third_number + some_fourth_number -
  some_fifth_number


I don't know if this last one is related or not, but it follows the same
rule plus the rule about blocks. Everything about the continuation of
the expression is indented once. The contents of the block are indented
once more. The end should line up with the line that opened the block.

Current:

some_variable = some_array.
                  map do |x|
  x + 1
end

Desired:

some_variable = some_array.
  map do |x|
    x + 1
  end

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

[Dmitry Gutov]

On 19/12/2022 04:54, Aaron Jensen wrote:
> 
> Follow-up to bug#60110

Thanks!

> I prefer rather simplictic indentation for Ruby (and this appears to be
> pretty common from codebases I've seen). Essentially, the rule is: If an
> expression continues on another line, indent it once.

FWIW, this feels a little wasteful -- working to emulate the editors 
which don't have much of a grammar definition, so they mostly line up 
things to the beginning of the previous line (plus maybe the indentation 
offset).

But I guess that can make some experience better when working in teams.

> Current:
> 
> some_variable = some_object.
>                    some_method
> 
> Desired:
> 
> some_variable = some_object.
>    some_method
> 
> Current:
> 
> some_variable = some_number + some_other_number *
>                                some_third_number + some_fourth_number -
>                  some_fifth_number
> 
> Desired:
> 
> some_variable = some_number + some_other_number *
>    some_third_number + some_fourth_number -
>    some_fifth_number

This was easier to change than I expected, so here's some patch 
attached. It's very WIP -- before moving it to release some 
reorganization of indentation rules is in order, to be able to put the 
new option in just one place, and to streamline how indentation after 
"." works.

This won't make it into 29.1, but we can put ruby-mode in ELPA after.

> I don't know if this last one is related or not, but it follows the same
> rule plus the rule about blocks. Everything about the continuation of
> the expression is indented once. The contents of the block are indented
> once more. The end should line up with the line that opened the block.
> 
> Current:
> 
> some_variable = some_array.
>                    map do |x|
>    x + 1
> end
> 
> Desired:
> 
> some_variable = some_array.
>    map do |x|
>      x + 1
>    end

This will take some more work too. Not in the least because the 
"Desired" forms looks illogical (at least in the context of SMIE): we're 
already "escaping" the current syntax node to line the indentation of 
the block to the beginning of the statement (which makes sense, at least 
from the ergonomics POV), so why would the line break matter?

Take more complex cases. How much indentation will the block have after 
some heterogeneous continuations? Is this right?

   some_variable = 4 +
     some_array.
       reduce do |acc, x|
         acc + x
       end

What if the continuations are all after the same operator?

   some_variable = 4 +
     some_var +
     some_array.reduce do |acc, x|
       acc + x
     end

   some_variable = some_var.
     some_method.
     map do |x|
       x + 1
     end

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 94 bytes --]

On 20/12/2022 04:12, Dmitry Gutov wrote:
> so here's some patch attached. It's very WIP

Here.

[-- Attachment #2: ruby-simplistic-indent-wip.diff --]
[-- Type: text/x-patch, Size: 1879 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 1f3e9b6ae7b..ecd88527aeb 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -608,10 +608,10 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
@@ -696,7 +696,7 @@ ruby-smie-rules
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
        (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
+       (cons 'column (+ (smie-indent-virtual)
                         ruby-indent-level))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
@@ -708,9 +708,10 @@ ruby-smie-rules
                      "<=>" ">" "<" ">=" "<=" "==" "===" "!=" "<<" ">>"
                      "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^=" "|"
                      "<<=" ">>=" "&&=" "||=" "and" "or"))
-     (and (smie-rule-parent-p ";" nil)
-          (smie-indent--hanging-p)
-          ruby-indent-level))
+     (if (and (smie-rule-parent-p ";" nil)
+              (smie-indent--hanging-p))
+         ruby-indent-level
+       (ruby-smie--indent-to-stmt ruby-indent-level)))
     (`(:before . "=")
      (save-excursion
       (and (smie-rule-parent-p " @ ")
@@ -725,6 +726,8 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . "iuwu-mod")
      (smie-rule-parent ruby-indent-level))
+    ('(:after . _)
+     (ruby-smie--indent-to-stmt ruby-indent-level))
     ))
 
 (defun ruby--at-indentation-p (&optional point)

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

[Aaron Jensen]

On Mon, Dec 19, 2022 at 9:12 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 19/12/2022 04:54, Aaron Jensen wrote:
> >
> > Follow-up to bug#60110
>
> Thanks!
>
> > I prefer rather simplictic indentation for Ruby (and this appears to be
> > pretty common from codebases I've seen). Essentially, the rule is: If an
> > expression continues on another line, indent it once.
>
> FWIW, this feels a little wasteful -- working to emulate the editors
> which don't have much of a grammar definition, so they mostly line up
> things to the beginning of the previous line (plus maybe the indentation
> offset).
>
> But I guess that can make some experience better when working in teams.

The implication here is that the current indentation
rules are somehow objectively better. I'd argue the opposite,
                                      that they have usability issues.

:)

In all seriousness though,

> > Current:
> >
> > some_variable = some_object.
> >                    some_method
> >
> > Desired:
> >
> > some_variable = some_object.
> >    some_method

This one isn't quite right, maybe there was an email formatting issue.
I'm expecting some_method to be indented by 1 level, for me it's 2:

some_variable = some_object.
    some_method

> >
> > Current:
> >
> > some_variable = some_number + some_other_number *
> >                                some_third_number + some_fourth_number -
> >                  some_fifth_number
> >
> > Desired:
> >
> > some_variable = some_number + some_other_number *
> >    some_third_number + some_fourth_number -
> >    some_fifth_number

This looks good.
>
> This was easier to change than I expected, so here's some patch
> attached. It's very WIP -- before moving it to release some
> reorganization of indentation rules is in order, to be able to put the
> new option in just one place, and to streamline how indentation after
> "." works.
>
> This won't make it into 29.1, but we can put ruby-mode in ELPA after.
>
> > I don't know if this last one is related or not, but it follows the same
> > rule plus the rule about blocks. Everything about the continuation of
> > the expression is indented once. The contents of the block are indented
> > once more. The end should line up with the line that opened the block.
> >
> > Current:
> >
> > some_variable = some_array.
> >                    map do |x|
> >    x + 1
> > end
> >
> > Desired:
> >
> > some_variable = some_array.
> >    map do |x|
> >      x + 1
> >    end
>
> This will take some more work too. Not in the least because the
> "Desired" forms looks illogical (at least in the context of SMIE): we're
> already "escaping" the current syntax node to line the indentation of
> the block to the beginning of the statement (which makes sense, at least
> from the ergonomics POV), so why would the line break matter?

Do you mean why are these different?

some_variable = some_array.
  map do |x|
    x + 1
  end

vs

some_variable = some_array.map do |x|
  x + 1
end

It's because the end is lined up with the line opened the
block/increased indentation. In the first example, the indented map
line is the beginning and on the second, it's the assignment.

>
> Take more complex cases. How much indentation will the block have after
> some heterogeneous continuations? Is this right?
>
>    some_variable = 4 +
>      some_array.
>        reduce do |acc, x|
>          acc + x
>        end

No, there is nothing that would cause reduce to be further indented.
It should be this:

some_variable = 4 +
  some_array.
  reduce do |acc, x|
    acc + x
  end

> What if the continuations are all after the same operator?
>
>    some_variable = 4 +
>      some_var +
>      some_array.reduce do |acc, x|
>        acc + x
>      end
>
>    some_variable = some_var.
>      some_method.
>      map do |x|
>        x + 1
>      end

Yes, these look right.

Thanks,

Aaron

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

[Aaron Jensen]

On Mon, Dec 19, 2022 at 11:48 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Mon, Dec 19, 2022 at 9:12 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >
> > On 19/12/2022 04:54, Aaron Jensen wrote:
> > >
> > > Follow-up to bug#60110
> >
> > Thanks!
> >
> > > I prefer rather simplictic indentation for Ruby (and this appears to be
> > > pretty common from codebases I've seen). Essentially, the rule is: If an
> > > expression continues on another line, indent it once.
> >
> > FWIW, this feels a little wasteful -- working to emulate the editors
> > which don't have much of a grammar definition, so they mostly line up
> > things to the beginning of the previous line (plus maybe the indentation
> > offset).
> >
> > But I guess that can make some experience better when working in teams.
>
> The implication here is that the current indentation
> rules are somehow objectively better. I'd argue the opposite,
>                                       that they have usability issues.
>
> :)
>
> In all seriousness though,

Sorry, I was going to say, this style of indentation is more inline
with what I see in the wild in Ruby codebases and essentially every
other editor I've seen. Also, enh-ruby-mode is what I'm using as a
guide here, and it has a pretty much perfect grasp of the grammar
since it uses the Ruby parser. It's just a simpler indentation norm.

Aaron

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

[Dmitry Gutov]

On 20/12/2022 07:56, Aaron Jensen wrote:
> On Mon, Dec 19, 2022 at 11:48 PM Aaron Jensen<aaronjensen@gmail.com>  wrote:
>> On Mon, Dec 19, 2022 at 9:12 PM Dmitry Gutov<dgutov@yandex.ru>  wrote:
>>> On 19/12/2022 04:54, Aaron Jensen wrote:
>>>> Follow-up to bug#60110
>>> Thanks!
>>>
>>>> I prefer rather simplictic indentation for Ruby (and this appears to be
>>>> pretty common from codebases I've seen). Essentially, the rule is: If an
>>>> expression continues on another line, indent it once.
>>> FWIW, this feels a little wasteful -- working to emulate the editors
>>> which don't have much of a grammar definition, so they mostly line up
>>> things to the beginning of the previous line (plus maybe the indentation
>>> offset).
>>>
>>> But I guess that can make some experience better when working in teams.
>> The implication here is that the current indentation
>> rules are somehow objectively better. I'd argue the opposite,
>>                                        that they have usability issues.
>>
>> 😄
>>
>> In all seriousness though,
> Sorry, I was going to say, this style of indentation is more inline
> with what I see in the wild in Ruby codebases and essentially every
> other editor I've seen. Also, enh-ruby-mode is what I'm using as a
> guide here, and it has a pretty much perfect grasp of the grammar
> since it uses the Ruby parser. It's just a simpler indentation norm.

I do believe it's "better" in the sense that can give more hints to the 
user WRT to the program structure -- which, unlike in Lisp, is not 
always obvious. For example, this:

   some_variable = some_number + some_other_number *
                                 some_third_number +
                   some_fourth_number -
                   some_fifth_number

conveys the implicit grouping, based on the operator precedence. It 
might be less important for * vs +, but others can be more obscure.

That's not to say that it must be everyone's cup of tea, or that you 
want it to look "lispy" every time. Ideally, there will always be a way 
to write the code, with default indentation config, that the result 
looks "mundane". E.g.

   some_variable = if true
                     2
                   else
                     3
                   end

=>

   some_variable =
     if true
       2
     else
       3
     end

(this is also customizable)

or

   some_method(foo,
               bar,
               baz)

=>

   some_method(
     foo,
     bar,
     baz
   )

And I'll be the first to admit that the current behavior still has some 
usability issues (though perhaps we'll disagree on the full list).

One example would be the previous bug report of yours, where the 
existing (the current default) behavior didn't really benefit anybody much.

Another -- expressions like

   some_method({
                 foo: bar,
                 tee: qux
               }, zzz)

where it's not 100% obvious what TRT is, but we could probably do better.

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

[Dmitry Gutov]

On 20/12/2022 06:48, Aaron Jensen wrote:

> In all seriousness though,
> 
>>> Current:
>>>
>>> some_variable = some_object.
>>>                     some_method
>>>
>>> Desired:
>>>
>>> some_variable = some_object.
>>>     some_method
> 
> This one isn't quite right, maybe there was an email formatting issue.
> I'm expecting some_method to be indented by 1 level, for me it's 2:
> 
> some_variable = some_object.
>      some_method

Yes, that's also the "needs more work" part.

>>>
>>> Current:
>>>
>>> some_variable = some_number + some_other_number *
>>>                                 some_third_number + some_fourth_number -
>>>                   some_fifth_number
>>>
>>> Desired:
>>>
>>> some_variable = some_number + some_other_number *
>>>     some_third_number + some_fourth_number -
>>>     some_fifth_number
> 
> This looks good.

The funny thing is this case looked more difficult originally.

>> This will take some more work too. Not in the least because the
>> "Desired" forms looks illogical (at least in the context of SMIE): we're
>> already "escaping" the current syntax node to line the indentation of
>> the block to the beginning of the statement (which makes sense, at least
>> from the ergonomics POV), so why would the line break matter?
> 
> Do you mean why are these different?
> 
> some_variable = some_array.
>    map do |x|
>      x + 1
>    end
> 
> vs
> 
> some_variable = some_array.map do |x|
>    x + 1
> end
> 
> It's because the end is lined up with the line opened the
> block/increased indentation. In the first example, the indented map
> line is the beginning and on the second, it's the assignment.

One might ask why it's lined up to 'map' only after it's moved to the 
next line, but not in the first example.

Anyway, the important part is to choose an unambiguous algorithm, even 
if it uses its own logic.

>> Take more complex cases. How much indentation will the block have after
>> some heterogeneous continuations? Is this right?
>>
>>     some_variable = 4 +
>>       some_array.
>>         reduce do |acc, x|
>>           acc + x
>>         end
> 
> No, there is nothing that would cause reduce to be further indented.
> It should be this:
> 
> some_variable = 4 +
>    some_array.
>    reduce do |acc, x|
>      acc + x
>    end

Okay, so there should be two basic cases:

- Indent to the beginning of the statement,
- If there was a line continuation (no matter how many), indent 1 extra 
level?

And maybe something related to parentheses, if used.

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

On 20/12/2022 18:19, Dmitry Gutov wrote:
> Okay, so there should be two basic cases:
> 
> - Indent to the beginning of the statement,
> - If there was a line continuation (no matter how many), indent 1 extra 
> level?

Here's another quick attempt to make this work without rewriting the 
rest of the logic.

Cases 1 and 2 seem to work now, but not the block example.

[-- Attachment #2: ruby-simplified-indent-v2.diff --]
[-- Type: text/x-patch, Size: 2777 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 1f3e9b6ae7b..02d2e0a2a75 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -285,6 +285,11 @@ ruby-method-params-indent
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
   :version "29.1")
 
+(defcustom ruby-indent-simplified t
+  "Foo bar."
+  :type 'boolean
+  :safe 'booleanp)
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -608,15 +613,20 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
       (memq (intern keyword) ruby-align-to-stmt-keywords)))
 
+(defconst ruby-smie--operators '("=" "+" "-" "*" "/" "&&" "||" "%" "**" "^" "&"
+                                 "<=>" ">" "<" ">=" "<=" "==" "===" "!=" "<<" ">>"
+                                 "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^=" "|"
+                                 "<<=" ">>=" "&&=" "||=" "and" "or"))
+
 (defun ruby-smie-rules (kind token)
   (pcase (cons kind token)
     ('(:elem . basic) ruby-indent-level)
@@ -684,6 +694,14 @@ ruby-smie-rules
            (cons 'column (current-column)))
        (smie-rule-parent (or ruby-method-params-indent 0))))
     ('(:before . "do") (ruby-smie--indent-to-stmt))
+    (`(:after . ,(pred (lambda (token)
+                         (and ruby-indent-simplified
+                              (or
+                               (equal token ".")
+                               (member token ruby-smie--operators))))))
+     (if (smie-indent--hanging-p)
+         (ruby-smie--indent-to-stmt ruby-indent-level)
+       (ruby-smie--indent-to-stmt)))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
          (when ruby-align-chained-calls
@@ -695,9 +713,11 @@ ruby-smie-rules
                    (goto-char (nth 1 parent))
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
-       (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
-                        ruby-indent-level))))
+       (if ruby-indent-simplified
+           (ruby-smie--indent-to-stmt ruby-indent-level)
+         (smie-backward-sexp ".")
+         (cons 'column (+ (current-column)
+                          ruby-indent-level)))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
     (`(:before . ,(or "when" "in"))

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

[Aaron Jensen]

On Tue, Dec 20, 2022 at 11:19 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 20/12/2022 06:48, Aaron Jensen wrote:
>
> > In all seriousness though,
> >
> >>> Current:
> >>>
> >>> some_variable = some_object.
> >>>                     some_method
> >>>
> >>> Desired:
> >>>
> >>> some_variable = some_object.
> >>>     some_method
> >
> > This one isn't quite right, maybe there was an email formatting issue.
> > I'm expecting some_method to be indented by 1 level, for me it's 2:
> >
> > some_variable = some_object.
> >      some_method
>
> Yes, that's also the "needs more work" part.

>    some_variable = some_number + some_other_number *
>                                 some_third_number +
>                   some_fourth_number -
>                   some_fifth_number

Yeah, with this I'd probably be trying to give a name to some of the
things (what is the name of the product there?) I don't think I've
ever seen code like that in practice to be honest.

>
> >>>
> >>> Current:
> >>>
> >>> some_variable = some_number + some_other_number *
> >>>                                 some_third_number + some_fourth_number -
> >>>                   some_fifth_number
> >>>
> >>> Desired:
> >>>
> >>> some_variable = some_number + some_other_number *
> >>>     some_third_number + some_fourth_number -
> >>>     some_fifth_number
> >
> > This looks good.
>
> The funny thing is this case looked more difficult originally.
>
> >> This will take some more work too. Not in the least because the
> >> "Desired" forms looks illogical (at least in the context of SMIE): we're
> >> already "escaping" the current syntax node to line the indentation of
> >> the block to the beginning of the statement (which makes sense, at least
> >> from the ergonomics POV), so why would the line break matter?
> >
> > Do you mean why are these different?
> >
> > some_variable = some_array.
> >    map do |x|
> >      x + 1
> >    end
> >
> > vs
> >
> > some_variable = some_array.map do |x|
> >    x + 1
> > end
> >
> > It's because the end is lined up with the line opened the
> > block/increased indentation. In the first example, the indented map
> > line is the beginning and on the second, it's the assignment.
>
> One might ask why it's lined up to 'map' only after it's moved to the
> next line, but not in the first example.

It's never lined up to map, I don't think that's the right way to
think about it. It's lined up to indent level 1. It isn't until after
the `end' that the indent level returns to 0.

Line continuation (mid-expression): +1 indent level
Block opening (mid-block): +1 indent level
Paren opening (mid-arguments/params): +1 indent level
And all the closing/endings: -1 indent level

Only one indent level can be added per line, so all that matters is
where the line ends. In short, there are a set of expressions that
require indentation if they span multiple lines:

expression-start
  expression-middle
expression-end

I haven't tried the patch yet, but I'll give it a shot.

Thank you,

Aaron

>
> Anyway, the important part is to choose an unambiguous algorithm, even
> if it uses its own logic.
>
> >> Take more complex cases. How much indentation will the block have after
> >> some heterogeneous continuations? Is this right?
> >>
> >>     some_variable = 4 +
> >>       some_array.
> >>         reduce do |acc, x|
> >>           acc + x
> >>         end
> >
> > No, there is nothing that would cause reduce to be further indented.
> > It should be this:
> >
> > some_variable = 4 +
> >    some_array.
> >    reduce do |acc, x|
> >      acc + x
> >    end
>
> Okay, so there should be two basic cases:
>
> - Indent to the beginning of the statement,
> - If there was a line continuation (no matter how many), indent 1 extra
> level?
>
> And maybe something related to parentheses, if used.

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

[Aaron Jensen]

On Tue, Dec 20, 2022 at 12:31 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 20/12/2022 18:19, Dmitry Gutov wrote:
> > Okay, so there should be two basic cases:
> >
> > - Indent to the beginning of the statement,
> > - If there was a line continuation (no matter how many), indent 1 extra
> > level?
>
> Here's another quick attempt to make this work without rewriting the
> rest of the logic.
>
> Cases 1 and 2 seem to work now, but not the block example.

I haven't had a chance to try this yet, but I just wanted to mention
that we put the .'s on the end of multi-line expressions, but putting
them at the beginning is probably more common:

some_variable = some_method
  .map { |x| x + 1 }
  .uniq

I just wanted to mention that in case it mattered for the work you are doing.

Thanks,

Aaron

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On 20/12/2022 22:05, Aaron Jensen wrote:
>>     some_variable = some_number + some_other_number *
>>                                  some_third_number +
>>                    some_fourth_number -
>>                    some_fifth_number
> 
> Yeah, with this I'd probably be trying to give a name to some of the
> things (what is the name of the product there?) I don't think I've
> ever seen code like that in practice to be honest.

Sure, but if such complex structures are not used, it also doesn't 
matter that the ruby-mode indents them differently from the "community 
baseline".

But it could still help when prototyping code, fiddling with the 
implementation (to factor pieces out into named variables later), etc.

>> One might ask why it's lined up to 'map' only after it's moved to the
>> next line, but not in the first example.
> 
> It's never lined up to map, I don't think that's the right way to
> think about it. It's lined up to indent level 1. It isn't until after
> the `end' that the indent level returns to 0.
> 
> Line continuation (mid-expression): +1 indent level
> Block opening (mid-block): +1 indent level
> Paren opening (mid-arguments/params): +1 indent level
> And all the closing/endings: -1 indent level
> 
> Only one indent level can be added per line, so all that matters is
> where the line ends. In short, there are a set of expressions that
> require indentation if they span multiple lines:
>
> expression-start
>    expression-middle
> expression-end

I think I got it. Only one indent level can be added for the duration of 
a statement. Unless there are nested blocks or parens/brackets/braces.

> I haven't tried the patch yet, but I'll give it a shot.

See this new patch instead.

The code is messier than I'd like it to be, but it seems to handle all 
of the cases mentioned so far and more (including the 
dots-at-indentation style, thanks).

[-- Attachment #2: ruby-simplified-indent-v3.diff --]
[-- Type: text/x-patch, Size: 7190 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 1f3e9b6ae7b..184ff6a61e0 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -285,6 +285,11 @@ ruby-method-params-indent
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
   :version "29.1")
 
+(defcustom ruby-indent-simplified t
+  "Foo bar."
+  :type 'boolean
+  :safe 'booleanp)
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -416,6 +421,7 @@ ruby-smie-grammar
      '((right "=")
        (right "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^="
               "<<=" ">>=" "&&=" "||=")
+       (right "?")
        (nonassoc ".." "...")
        (left "&&" "||")
        (nonassoc "<=>")
@@ -608,10 +614,22 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
+
+(defun ruby-smie--indent-with-continuation (token)
+  (let* (indent
+         (stmt-beg (save-excursion
+                     (smie-backward-sexp ";")
+                     (setq indent (smie-indent-virtual))
+                     (point)))
+         (nls (1- (count-lines stmt-beg (point)))))
+    (when (and (not (equal token "do")) (smie-indent--hanging-p))
+      (cl-incf nls))
+    (when (> nls 0) (cl-incf indent ruby-indent-level))
+    (cons 'column indent)))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
@@ -642,7 +660,9 @@ ruby-smie-rules
               (forward-comment -1)
               (not (eq (preceding-char) ?:))))
        ;; Curly block opener.
-       (ruby-smie--indent-to-stmt))
+       (if ruby-indent-simplified
+           (ruby-smie--indent-with-continuation token)
+         (ruby-smie--indent-to-stmt)))
       ((smie-rule-hanging-p)
        ;; Treat purely syntactic block-constructs as being part of their parent,
        ;; when the opening token is hanging and the parent is not an
@@ -683,7 +703,6 @@ ruby-smie-rules
            (skip-chars-forward " \t")
            (cons 'column (current-column)))
        (smie-rule-parent (or ruby-method-params-indent 0))))
-    ('(:before . "do") (ruby-smie--indent-to-stmt))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
          (when ruby-align-chained-calls
@@ -696,8 +715,10 @@ ruby-smie-rules
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
        (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
-                        ruby-indent-level))))
+       (if ruby-indent-simplified
+           (ruby-smie--indent-to-stmt ruby-indent-level)
+         (cons 'column (+ (current-column)
+                          ruby-indent-level)))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
     (`(:before . ,(or "when" "in"))
@@ -710,14 +731,16 @@ ruby-smie-rules
                      "<<=" ">>=" "&&=" "||=" "and" "or"))
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
-          ruby-indent-level))
+          (if ruby-indent-simplified
+              (ruby-smie--indent-to-stmt ruby-indent-level)
+            ruby-indent-level)))
     (`(:before . "=")
      (save-excursion
       (and (smie-rule-parent-p " @ ")
            (goto-char (nth 1 (smie-indent--parent)))
            (smie-rule-prev-p "def=")
            (cons 'column (+ (current-column) ruby-indent-level -3)))))
-    (`(:after . ,(or "?" ":")) ruby-indent-level)
+    (`(:after . ,(or "?" ":")) (unless ruby-indent-simplified ruby-indent-level))
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
        (if (ruby-smie--indent-to-stmt-p token)
@@ -725,7 +748,18 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . "iuwu-mod")
      (smie-rule-parent ruby-indent-level))
-    ))
+    (`(:before . ,_)
+     (when (and ruby-indent-simplified
+                (not (or (member token '(","))
+                         (smie-rule-prev-p ";"))))
+       (let* ((stmt-beg (save-excursion
+                          (smie-backward-sexp ";")
+                          (point)))
+              (nls (1- (count-lines stmt-beg (point)))))
+         (when (and (not (equal token "do")) (smie-indent--hanging-p))
+           (cl-incf nls))
+         (ruby-smie--indent-to-stmt (if (> nls 0) ruby-indent-level 0)))))
+    ('(:before . "do") (ruby-smie--indent-to-stmt))))
 
 (defun ruby--at-indentation-p (&optional point)
   (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index 6a69d9db78a..3bf35790099 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -185,14 +185,14 @@ def test2 (arg)
 
 # Example from https://ruby-doc.com/docs/ProgrammingRuby/
 d = 4 + 5 +      # no '\' needed
-    6 + 7
+  6 + 7
 
 # Example from https://www.ruby-doc.org/docs/ProgrammingRuby/
 e = 8 + 9   \
-    + 10         # '\' needed
+  + 10         # '\' needed
 
 foo = obj.bar { |m| tee(m) } +
-      obj.qux { |m| hum(m) }
+  obj.qux { |m| hum(m) }
 
 begin
   foo
@@ -215,7 +215,7 @@ def begin
 end
 
 a = foo(j, k) -
-    bar_tee
+  bar_tee
 
 while a < b do # "do" is optional
   foo
@@ -224,8 +224,8 @@ def begin
 desc "foo foo" \
      "bar bar"
 
-foo.
-  bar
+foo
+  .bar
 
 # https://github.com/rails/rails/blob/17f5d8e062909f1fcae25351834d8e89967b645e/activesupport/lib/active_support/time_with_zone.rb#L206
 foo # comment intended to confuse the tokenizer
@@ -288,7 +288,7 @@ def begin
 }
 
 if foo &&
-   bar
+     bar
 end
 
 foo +
@@ -312,10 +312,10 @@ def begin
   tee + qux
 
 1 .. 2 &&
-     3
+  3
 
 3 < 4 +
-    5
+  5
 
 10 << 4 ^
   20
@@ -418,8 +418,9 @@ def qux
 ddd
 
 qux = foo.fee ?
-        bar :
-        tee
+  bar + 3 *
+  4 :
+  tee
 
 zoo.keep.bar!(
   {x: y,
@@ -439,9 +440,9 @@ def qux
 
 foo2 =
   subject.
-    update(
-      2
-    )
+  update(
+    2
+  )
 
 # FIXME: This is not consistent with the example below it, but this
 # offset only happens if the colon is at eol, which wouldn't be often.
@@ -451,7 +452,7 @@ def qux
       tee)
 
 foo(:bar =>
-    tee)
+      tee)
 
 regions = foo(
   OpenStruct.new(id: 0, name: "foo") => [
@@ -500,9 +501,17 @@ def qux
 
 # Tokenizing "**" and "|" separately.
 def resolve(**args)
-  members = proc do |**args|
-    p(**args)
-  end
+  members = foo
+    .asdasd
+    .proc do |**args|
+      p(**args)
+    end
+
+  members = foo
+    .asdasd
+    .proc { |**args|
+      p(**args)
+    }
 
   member.call(**args)
 end
@@ -510,7 +519,7 @@ def resolve(**args)
 # Endless methods.
 class Bar
   def foo(abc) = bar +
-                 baz
+    baz
 
   def self.bar =
     123 +
@@ -541,4 +550,5 @@ def baz.full_name = "#{bar} 3"
 
 # Local Variables:
 # ruby-method-params-indent: t
+# ruby-indent-simplified: t
 # End:

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

[Aaron Jensen]

On Wed, Dec 21, 2022 at 5:48 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> See this new patch instead.
>
> The code is messier than I'd like it to be, but it seems to handle all
> of the cases mentioned so far and more (including the
> dots-at-indentation style, thanks).

Looks good, for the things I mentioned. I found one more case:

x.foo do
  foo
end.bar do
    bar
  end

Should be:

x.foo do
  foo
end.bar do
  bar
end


I can't vouch for writing in this style, but it should only get one
indentation increase in this instance, rather than, I believe.

Interestingly enough, I found a bug with enh-ruby-mode that ruby-mode
now indents correctly:

x =
  bar(
    y
  ).map do |i|
  i
  end

Thanks,

Aaron

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On 22/12/2022 04:31, Aaron Jensen wrote:
> On Wed, Dec 21, 2022 at 5:48 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>> See this new patch instead.
>>
>> The code is messier than I'd like it to be, but it seems to handle all
>> of the cases mentioned so far and more (including the
>> dots-at-indentation style, thanks).
> 
> Looks good, for the things I mentioned. I found one more case:
> 
> x.foo do
>    foo
> end.bar do
>      bar
>    end
> 
> Should be:
> 
> x.foo do
>    foo
> end.bar do
>    bar
> end

Hm, this one breaks the approach I used with the last patch (which was 
to count lines from the beginning of the statement).

Let's see if blocks can just be aligned to the indentation of its 
opener's line.

> I can't vouch for writing in this style, but it should only get one
> indentation increase in this instance, rather than, I believe.
> 
> Interestingly enough, I found a bug with enh-ruby-mode that ruby-mode
> now indents correctly:
> 
> x =
>    bar(
>      y
>    ).map do |i|
>    i
>    end

Cool. It seems I broke it for the default indent algo, however. ;-( In 
the previous patch.

See the new one attached.

BTW, I'm surprised you haven't mentioned the case of parenless calls:

foo bar,
     baz,
     tee

IUUC the Rails core has decided to forgo this indentation style. Not 
sure about the statistics across other popular projects.

[-- Attachment #2: ruby-simplified-indent-v4.diff --]
[-- Type: text/x-patch, Size: 6710 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 1f3e9b6ae7b..542c8ac02f1 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -285,6 +285,11 @@ ruby-method-params-indent
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
   :version "29.1")
 
+(defcustom ruby-indent-simplified nil
+  "Foo bar."
+  :type 'boolean
+  :safe 'booleanp)
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -416,6 +421,7 @@ ruby-smie-grammar
      '((right "=")
        (right "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^="
               "<<=" ">>=" "&&=" "||=")
+       (right "?")
        (nonassoc ".." "...")
        (left "&&" "||")
        (nonassoc "<=>")
@@ -608,10 +614,10 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
@@ -642,7 +648,9 @@ ruby-smie-rules
               (forward-comment -1)
               (not (eq (preceding-char) ?:))))
        ;; Curly block opener.
-       (ruby-smie--indent-to-stmt))
+       (if ruby-indent-simplified
+           (cons 'column (current-indentation))
+         (ruby-smie--indent-to-stmt)))
       ((smie-rule-hanging-p)
        ;; Treat purely syntactic block-constructs as being part of their parent,
        ;; when the opening token is hanging and the parent is not an
@@ -683,7 +691,10 @@ ruby-smie-rules
            (skip-chars-forward " \t")
            (cons 'column (current-column)))
        (smie-rule-parent (or ruby-method-params-indent 0))))
-    ('(:before . "do") (ruby-smie--indent-to-stmt))
+    ('(:before . "do")
+     (if ruby-indent-simplified
+         (cons 'column (current-indentation))
+       (ruby-smie--indent-to-stmt)))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
          (when ruby-align-chained-calls
@@ -696,8 +707,10 @@ ruby-smie-rules
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
        (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
-                        ruby-indent-level))))
+       (if ruby-indent-simplified
+           (ruby-smie--indent-to-stmt ruby-indent-level)
+         (cons 'column (+ (current-column)
+                          ruby-indent-level)))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
     (`(:before . ,(or "when" "in"))
@@ -710,14 +723,16 @@ ruby-smie-rules
                      "<<=" ">>=" "&&=" "||=" "and" "or"))
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
-          ruby-indent-level))
+          (if ruby-indent-simplified
+              (ruby-smie--indent-to-stmt ruby-indent-level)
+            ruby-indent-level)))
     (`(:before . "=")
      (save-excursion
       (and (smie-rule-parent-p " @ ")
            (goto-char (nth 1 (smie-indent--parent)))
            (smie-rule-prev-p "def=")
            (cons 'column (+ (current-column) ruby-indent-level -3)))))
-    (`(:after . ,(or "?" ":")) ruby-indent-level)
+    (`(:after . ,(or "?" ":")) (unless ruby-indent-simplified ruby-indent-level))
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
        (if (ruby-smie--indent-to-stmt-p token)
@@ -725,7 +740,17 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . "iuwu-mod")
      (smie-rule-parent ruby-indent-level))
-    ))
+    (`(:before . ,_)
+     (when (and ruby-indent-simplified
+                (not (or (member token '(","))
+                         (smie-rule-prev-p ";"))))
+       (let* ((stmt-beg (save-excursion
+                          (smie-backward-sexp ";")
+                          (point)))
+              (nls (1- (count-lines stmt-beg (point)))))
+         (when (smie-indent--hanging-p)
+           (cl-incf nls))
+         (ruby-smie--indent-to-stmt (if (> nls 0) ruby-indent-level 0)))))))
 
 (defun ruby--at-indentation-p (&optional point)
   (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb
new file mode 100644
index 00000000000..9f1fb0edd80
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb
@@ -0,0 +1,62 @@
+4 +
+  5 +
+  6 +
+  7
+
+foo = obj.bar { |m| tee(m) } +
+  obj.qux { |m| hum(m) }
+
+foo.
+  bar
+  .baz
+
+qux = foo.fee ?
+  bar :
+  tee
+
+foo2 =
+  subject.
+  update(
+    2
+  )
+
+m1 = foo
+  .asdasd
+  .proc do |**args|
+    p(**args)
+  end
+
+m2 = foo
+  .asdasd
+  .proc { |**args|
+    p(**args)
+  }
+
+bar.foo do
+  bar
+end
+
+bar.foo(tee) do
+  bar
+end
+
+bar.foo(tee) {
+  bar
+}
+
+# Endless methods.
+class Bar
+  def foo(abc) = bar +
+    baz
+end
+
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
+# Local Variables:
+# ruby-method-params-indent: t
+# ruby-indent-simplified: t
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index 6a69d9db78a..d0ee8f8f52b 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -226,6 +226,7 @@ def begin
 
 foo.
   bar
+  .baz
 
 # https://github.com/rails/rails/blob/17f5d8e062909f1fcae25351834d8e89967b645e/activesupport/lib/active_support/time_with_zone.rb#L206
 foo # comment intended to confuse the tokenizer
@@ -380,6 +381,18 @@ def bar
   i + 1
 end
 
+m1 = foo
+       .asdasd
+       .proc do |**args|
+  p(**args)
+end
+
+m2 = foo
+       .asdasd
+       .proc { |**args|
+  p(**args)
+}
+
 bar.foo do
   bar
 end
@@ -398,6 +411,12 @@ def bar
   end
 end
 
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
 foo |
   bar
 
@@ -541,4 +560,5 @@ def baz.full_name = "#{bar} 3"
 
 # Local Variables:
 # ruby-method-params-indent: t
+# ruby-indent-simplified: nil
 # End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 560f780285a..4a86492b137 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -957,6 +957,7 @@ ruby-deftest-indent
 
 (ruby-deftest-indent "ruby.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
+(ruby-deftest-indent "ruby-indent-simplified.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer

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

[Aaron Jensen]

On Thu, Dec 22, 2022 at 4:21 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 22/12/2022 04:31, Aaron Jensen wrote:
> > On Wed, Dec 21, 2022 at 5:48 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >> See this new patch instead.
> >>
> >> The code is messier than I'd like it to be, but it seems to handle all
> >> of the cases mentioned so far and more (including the
> >> dots-at-indentation style, thanks).
> >
> > Looks good, for the things I mentioned. I found one more case:
> >
> > x.foo do
> >    foo
> > end.bar do
> >      bar
> >    end
> >
> > Should be:
> >
> > x.foo do
> >    foo
> > end.bar do
> >    bar
> > end
>
> Hm, this one breaks the approach I used with the last patch (which was
> to count lines from the beginning of the statement).
>
> Let's see if blocks can just be aligned to the indentation of its
> opener's line.
>
> > I can't vouch for writing in this style, but it should only get one
> > indentation increase in this instance, rather than, I believe.
> >
> > Interestingly enough, I found a bug with enh-ruby-mode that ruby-mode
> > now indents correctly:
> >
> > x =
> >    bar(
> >      y
> >    ).map do |i|
> >    i
> >    end
>
> Cool. It seems I broke it for the default indent algo, however. ;-( In
> the previous patch.
>
> See the new one attached.

Seems to work well with everything I threw at it.

> BTW, I'm surprised you haven't mentioned the case of parenless calls:
>
> foo bar,
>      baz,
>      tee
>
> IUUC the Rails core has decided to forgo this indentation style. Not
> sure about the statistics across other popular projects.

I try to avoid this style in general. The simplified style with the 2
spaces means the first argument is on a different plane than the rest
which negatively impacts scanning. 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".
This means that any time we decide to split a method invocation on
multiple lines we use the basic newline after ( style.

Aaron

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

[Dmitry Gutov]

On 23/12/2022 06:12, Aaron Jensen wrote:

>> See the new one attached.
> 
> Seems to work well with everything I threw at it.

Thank you. I think I'll tinker with it and then push to master sometime 
later. Maybe after 29 RC is cut, or if I get to a nicer rewrite earlier.

>> BTW, I'm surprised you haven't mentioned the case of parenless calls:
>>
>> foo bar,
>>       baz,
>>       tee
>>
>> IUUC the Rails core has decided to forgo this indentation style. Not
>> sure about the statistics across other popular projects.
> 
> I try to avoid this style in general.

Is that also true for the other "codebases you've seen" referred to in 
the first message here?

> The simplified style with the 2
> spaces means the first argument is on a different plane than the rest
> which negatively impacts scanning.

Makes sense.

> 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 checked out some popular projects out there. Rails' style is 
inconsistent, with class-level parenless calls lined up vertically and 
the rare use of them inside methods seem to go the "simplified" route. 
I'm not sure if that's intentional, or whether it's just written by 
different people.

Redmine mixes styles, but mostly on the side of lining up.

Spree lines up and overall seems to vindicate the default "lispy" 
indentation style, e.g. here:

https://github.com/spree/spree/blob/main/core/app/models/spree/zone.rb
https://github.com/spree/spree/blob/main/core/app/models/spree/tax_rate.rb

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.

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

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

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

[Aaron Jensen]

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

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

> I checked out some popular projects out there. Rails' style is
> inconsistent, with class-level parenless calls lined up vertically and
> the rare use of them inside methods seem to go the "simplified" route.
> I'm not sure if that's intentional, or whether it's just written by
> different people.
>
> Redmine mixes styles, but mostly on the side of lining up.
>
> Spree lines up and overall seems to vindicate the default "lispy"
> indentation style, e.g. here:
>
> https://github.com/spree/spree/blob/main/core/app/models/spree/zone.rb
> https://github.com/spree/spree/blob/main/core/app/models/spree/tax_rate.rb
>
> 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.

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

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

Aaron

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 4197 bytes --]

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.

[-- Attachment #2: ruby-simplified-indent-v5.diff --]
[-- Type: text/x-patch, Size: 8383 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index a4aa61905e4..2fe1f4986ee 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -285,6 +285,40 @@ ruby-method-params-indent
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
   :version "29.1")
 
+(defcustom ruby-block-indent t
+  "Non-nil to align the body of a block to the statement's start.
+
+The body and the closer will be aligned to the column where the
+statement containing the block starts. Example:
+
+  foo.bar
+    .each do
+    baz
+  end
+
+If nil, it will be aligned instead to the beginning of the line
+containing the block's opener:
+
+  foo.bar
+    .each do
+      baz
+    end
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
+(defcustom ruby-indent-simplified nil
+  "Non-nil to use the indentation logic with less nesting.
+
+The result is that statements with continuations will have just 1
+level of indentation nesting.  Unless paren grouping or
+hash/array literals are involved.
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -416,6 +450,7 @@ ruby-smie-grammar
      '((right "=")
        (right "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^="
               "<<=" ">>=" "&&=" "||=")
+       (right "?")
        (nonassoc ".." "...")
        (left "&&" "||")
        (nonassoc "<=>")
@@ -608,10 +643,10 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
@@ -642,7 +677,9 @@ ruby-smie-rules
               (forward-comment -1)
               (not (eq (preceding-char) ?:))))
        ;; Curly block opener.
-       (ruby-smie--indent-to-stmt))
+       (if ruby-block-indent
+           (ruby-smie--indent-to-stmt)
+         (cons 'column (current-indentation))))
       ((smie-rule-hanging-p)
        ;; Treat purely syntactic block-constructs as being part of their parent,
        ;; when the opening token is hanging and the parent is not an
@@ -678,12 +715,16 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . " @ ")
      (if (or (eq ruby-method-params-indent t)
+             (eq ruby-indent-simplified nil)
              (not (smie-rule-parent-p "def" "def=")))
          (save-excursion
            (skip-chars-forward " \t")
            (cons 'column (current-column)))
        (smie-rule-parent (or ruby-method-params-indent 0))))
-    ('(:before . "do") (ruby-smie--indent-to-stmt))
+    ('(:before . "do")
+     (if ruby-block-indent
+         (ruby-smie--indent-to-stmt)
+       (cons 'column (current-indentation))))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
          (when ruby-align-chained-calls
@@ -696,8 +737,10 @@ ruby-smie-rules
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
        (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
-                        ruby-indent-level))))
+       (if ruby-indent-simplified
+           (ruby-smie--indent-to-stmt ruby-indent-level)
+         (cons 'column (+ (current-column)
+                          ruby-indent-level)))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
     (`(:before . ,(or "when" "in"))
@@ -710,14 +753,16 @@ ruby-smie-rules
                      "<<=" ">>=" "&&=" "||=" "and" "or"))
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
-          ruby-indent-level))
+          (if ruby-indent-simplified
+              (ruby-smie--indent-to-stmt ruby-indent-level)
+            ruby-indent-level)))
     (`(:before . "=")
      (save-excursion
       (and (smie-rule-parent-p " @ ")
            (goto-char (nth 1 (smie-indent--parent)))
            (smie-rule-prev-p "def=")
            (cons 'column (+ (current-column) ruby-indent-level -3)))))
-    (`(:after . ,(or "?" ":")) ruby-indent-level)
+    (`(:after . ,(or "?" ":")) (unless ruby-indent-simplified ruby-indent-level))
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
        (if (ruby-smie--indent-to-stmt-p token)
@@ -725,7 +770,21 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . "iuwu-mod")
      (smie-rule-parent ruby-indent-level))
-    ))
+    (`(:before . ",")
+     (and ruby-indent-simplified
+          (smie-rule-parent-p " @ ")
+          (ruby-smie--indent-to-stmt ruby-indent-level)))
+    (`(:before . ,_)
+     (when (and ruby-indent-simplified
+                (not (or (member token '(","))
+                         (smie-rule-prev-p ";"))))
+       (let* ((stmt-beg (save-excursion
+                          (smie-backward-sexp ";")
+                          (point)))
+              (nls (1- (count-lines stmt-beg (point)))))
+         (when (smie-indent--hanging-p)
+           (cl-incf nls))
+         (ruby-smie--indent-to-stmt (if (> nls 0) ruby-indent-level 0)))))))
 
 (defun ruby--at-indentation-p (&optional point)
   (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb
new file mode 100644
index 00000000000..03acdda6fb0
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb
@@ -0,0 +1,15 @@
+foo
+  .asdasd
+  .proc do |**args|
+    p(**args)
+  end
+
+foo
+  .asdasd
+  .proc { |**args|
+    p(**args)
+  }
+
+# Local Variables:
+# ruby-block-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb
new file mode 100644
index 00000000000..aaecc7f7422
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-indent-simplified.rb
@@ -0,0 +1,61 @@
+4 +
+  5 +
+  6 +
+  7
+
+foo = obj.bar { |m| tee(m) } +
+  obj.qux { |m| hum(m) }
+
+foo(a,
+    b)
+
+foo.
+  bar
+  .baz
+
+method arg1,                   # bug#15594
+  method2 arg2,
+  arg3
+
+zzz = method (a + b),
+  c, :d => :e,
+  f: g
+
+qux = foo.fee ?
+  bar :
+  tee
+
+foo2 =
+  subject.
+  update(
+    2
+  )
+
+bar.foo do
+  bar
+end
+
+bar.foo(tee) do
+  bar
+end
+
+bar.foo(tee) {
+  bar
+}
+
+# Endless methods.
+class Bar
+  def foo(abc) = bar +
+    baz
+end
+
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
+# Local Variables:
+# ruby-method-params-indent: t
+# ruby-indent-simplified: t
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index 6a69d9db78a..7f2a665f2f4 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -226,6 +226,7 @@ def begin
 
 foo.
   bar
+  .baz
 
 # https://github.com/rails/rails/blob/17f5d8e062909f1fcae25351834d8e89967b645e/activesupport/lib/active_support/time_with_zone.rb#L206
 foo # comment intended to confuse the tokenizer
@@ -380,6 +381,18 @@ def bar
   i + 1
 end
 
+m1 = foo
+       .asdasd
+       .proc do |**args|
+  p(**args)
+end
+
+m2 = foo
+       .asdasd
+       .proc { |**args|
+  p(**args)
+}
+
 bar.foo do
   bar
 end
@@ -398,6 +411,12 @@ def bar
   end
 end
 
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
 foo |
   bar
 
@@ -541,4 +560,6 @@ def baz.full_name = "#{bar} 3"
 
 # Local Variables:
 # ruby-method-params-indent: t
+# ruby-indent-simplified: nil
+# ruby-block-indent: t
 # End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 560f780285a..3b0ec8a0324 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -957,6 +957,8 @@ ruby-deftest-indent
 
 (ruby-deftest-indent "ruby.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
+(ruby-deftest-indent "ruby-indent-simplified.rb")
+(ruby-deftest-indent "ruby-block-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer

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

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

I forgot to mention that I tried the patch out and it seems to work well with:

(setq ruby-indent-simplified t
      ruby-block-indent nil)

Those are the only two I should be setting, correct?

Thanks,

Aaron

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

[Dmitry Gutov]

X-Debbugs-CC: aaronjensen@gmail.com

Splitting off from debbugs#60186.

Since this setting also seems orthogonal to the "simplified" preference, 
and it'll require some more work.

On 25/12/2022 02:12, Aaron Jensen wrote:
>> 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
> ]
> )

Here are a couple trickier examples:

takes_multi_pairs_hash(x: {
   a: 1,
   b: 2
})

and_in_a_method_call({
   no: :difference
},
foo,
bar)

AFAICT even Rubocop doesn't have a setting which would indent the second 
one somewhat reasonably, while keeping two-space indent before "no".

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

Yup, that looks pretty bizarre. OTOH, I don't see why a developer would 
put a newline between "]" and ")" in this case.

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

[Dmitry Gutov]

X-Debbugs-CC: aaronjensen@gmail.com

Splitting off from debbugs#60186, second try.

Since this setting also seems orthogonal to the "simplified" preference, 
and it'll require some more work.

For future reference, here are the relevant Rubocop settings:

https://docs.rubocop.org/rubocop/cops_layout.html#layoutfirsthashelementindentation
https://docs.rubocop.org/rubocop/cops_layout.html#layoutfirstarrayelementindentation

On 25/12/2022 02:12, Aaron Jensen wrote:
>> 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
> ]
> )

Here are a couple trickier examples:

takes_multi_pairs_hash(x: {
   a: 1,
   b: 2
})

and_in_a_method_call({
   no: :difference
},
foo,
bar)

AFAICT even Rubocop doesn't have a setting which would indent the second 
one somewhat reasonably, while keeping two-space indent before "no".

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

Yup, that looks pretty bizarre. OTOH, I don't see why a developer would 
put a newline between "]" and ")" in this case.

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

[Dmitry Gutov]

On 25/12/2022 02:14, Aaron Jensen wrote:
> On Sat, Dec 24, 2022 at 5:47 PM Dmitry Gutov<dgutov@yandex.ru>  wrote:
>> 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.
> I forgot to mention that I tried the patch out and it seems to work well with:
> 
> (setq ruby-indent-simplified t
>        ruby-block-indent nil)
> 
> Those are the only two I should be setting, correct?

Yep.

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

[Aaron Jensen]

On Sun, Dec 25, 2022 at 4:30 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> X-Debbugs-CC: aaronjensen@gmail.com
>
> Splitting off from debbugs#60186, second try.
>
> Since this setting also seems orthogonal to the "simplified" preference,
> and it'll require some more work.
>
> For future reference, here are the relevant Rubocop settings:
>
> https://docs.rubocop.org/rubocop/cops_layout.html#layoutfirsthashelementindentation
> https://docs.rubocop.org/rubocop/cops_layout.html#layoutfirstarrayelementindentation
>
> On 25/12/2022 02:12, Aaron Jensen wrote:
> >> 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
> > ]
> > )
>
> Here are a couple trickier examples:
>
> takes_multi_pairs_hash(x: {
>    a: 1,
>    b: 2
> })

enh-ruby-mode and vim do the same thing, which I think is fine:

takes_multi_pairs_hash(x: {
  a: 1,
  b: 2
})

Though again, the best answer imo is "don't do this".


>
> and_in_a_method_call({
>    no: :difference
> },
> foo,
> bar)

enh-ruby-mode:

and_in_a_method_call({
  no: :difference
},
                     foo,
                     bar)


Vim:

and_in_a_method_call({
  no: :difference
},
foo,
bar)

I think this falls under something I wouldn't put too much effort into
fixing. I would write it like this:

and_in_a_method_call(
  {
    no: :difference
  },
  foo,
  bar
)

Which indents in a straightforward manner.

If I had to type it as above, I would probably indent it like this:

and_in_a_method_call({
    no: :difference
  },
  foo,
  bar)

But I can't imagine that would be easy to implement at all, so I
wouldn't bother.


> AFAICT even Rubocop doesn't have a setting which would indent the second
> one somewhat reasonably, while keeping two-space indent before "no".
>
> > 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
> > ]
> >     )
>
> Yup, that looks pretty bizarre. OTOH, I don't see why a developer would
> put a newline between "]" and ")" in this case.

Exactly, that's what I meant by a social problem. We have a (somewhat
harsh) saying for stuff like this: you get what you deserve. That's
actually why I don't mind enh-ruby-mode's behavior here. It's clearly
undefined/out of bounds, so that tells a person they are currently out
of bounds and they should get back in bounds.

Aaron

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

[Dmitry Gutov]

On 26/12/2022 01:46, Aaron Jensen wrote:

> enh-ruby-mode and vim do the same thing, which I think is fine:
> 
> takes_multi_pairs_hash(x: {
>    a: 1,
>    b: 2
> })

Makes sense to me, I just wanted to keep this case in the bug report 
because it will likely need a separate indentation rule or a separate 
clause.

>> and_in_a_method_call({
>>     no: :difference
>> },
>> foo,
>> bar)
> 
> enh-ruby-mode:
> 
> and_in_a_method_call({
>    no: :difference
> },
>                       foo,
>                       bar)
> 
> 
> Vim:
> 
> and_in_a_method_call({
>    no: :difference
> },
> foo,
> bar)

Vim's choice looks saner to my eye. Probably comes down to the choice of 
indentation algorithm, though.

> I think this falls under something I wouldn't put too much effort into
> fixing. I would write it like this:
> 
> and_in_a_method_call(
>    {
>      no: :difference
>    },
>    foo,
>    bar
> )
> 
> Which indents in a straightforward manner.

Indeed. But this works already, no changes required.

> If I had to type it as above, I would probably indent it like this:
> 
> and_in_a_method_call({
>      no: :difference
>    },
>    foo,
>    bar)
> 
> But I can't imagine that would be easy to implement at all, so I
> wouldn't bother.

The indentation logic itself might be not that difficult to write, but 
the fact that the expression will have to be reindented as soon as the 
method call grows a second argument (after the user types the comma?), 
makes it a hard sell usability-wise.

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

[Dmitry Gutov]

On 25/12/2022 02:14, Aaron Jensen wrote:
> (setq ruby-indent-simplified t

BTW, do you have any opinion on the name? Perhaps something more 
semantic would be easier to discover.

A recent tree-sitter thread brought up sh-indent-after-continuation. 
It's not a direct counterpart, though, and the examples only look 
remotely similar.

Call ours ruby-indent-continuations-simplified, maybe? Now that we seem 
to have reduced its scope to expression continuations across newlines.

Hopefully it won't be confused with Kernel#callcc.

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

[Aaron Jensen]

On Mon, Dec 26, 2022 at 8:16 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Vim's choice looks saner to my eye. Probably comes down to the choice of
> indentation algorithm, though.

Agreed, though it's hard to pick which is more sane when all the
options start with insanity.

> > If I had to type it as above, I would probably indent it like this:
> >
> > and_in_a_method_call({
> >      no: :difference
> >    },
> >    foo,
> >    bar)
> >
> > But I can't imagine that would be easy to implement at all, so I
> > wouldn't bother.
>
> The indentation logic itself might be not that difficult to write, but
> the fact that the expression will have to be reindented as soon as the
> method call grows a second argument (after the user types the comma?),
> makes it a hard sell usability-wise.

Right, I think that's just more of the same thing... We are looking at
ways of writing code that are out of the realm of reason. It's a
challenge to define behavior when the behavior could very well be
undefined. But, if we must define it, then what are our guiding
principles? Not having to reindent preceding lines when adding a new
line may be a very reasonable one. In that case, the only two options
I could think of would be:

and_in_a_method_call({
  no: :difference
  },
  foo,
  bar)

or

and_in_a_method_call({
  no: :difference
  },
foo,
bar)

The difference being if we decide to dedent upon the last closing
indent-requiring-token or the first.

I think a reasonable rule of thumb for a human might be: "If you open
N indents on one line, you must close N indents on one line". Any time
you stray away from this, behavior becomes... not ideal.

Aaron

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

[Aaron Jensen]

On Mon, Dec 26, 2022 at 8:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 25/12/2022 02:14, Aaron Jensen wrote:
> > (setq ruby-indent-simplified t
>
> BTW, do you have any opinion on the name? Perhaps something more
> semantic would be easier to discover.
>
> A recent tree-sitter thread brought up sh-indent-after-continuation.
> It's not a direct counterpart, though, and the examples only look
> remotely similar.
>
> Call ours ruby-indent-continuations-simplified, maybe? Now that we seem
> to have reduced its scope to expression continuations across newlines.
>
> Hopefully it won't be confused with Kernel#callcc.

Simple is what it is in comparison to something more complex. All
indentations are pretty much about line continuation in one way or
another.

What is it on its own? I'm not sure.

Some food for thought:

Unaligned
Beginning of line aligned
Standard
Incremental
Snap-to-grid
K&R style
C style
Egyptian brackets
Not Lisp style
Lisp alignment: false
Argument alignment: false

Sort of a smattering there, but maybe it'll jog something for you.

Aaron

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

[Dmitry Gutov]

On 27/12/2022 03:47, Aaron Jensen wrote:
> On Mon, Dec 26, 2022 at 8:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> On 25/12/2022 02:14, Aaron Jensen wrote:
>>> (setq ruby-indent-simplified t
>>
>> BTW, do you have any opinion on the name? Perhaps something more
>> semantic would be easier to discover.
>>
>> A recent tree-sitter thread brought up sh-indent-after-continuation.
>> It's not a direct counterpart, though, and the examples only look
>> remotely similar.
>>
>> Call ours ruby-indent-continuations-simplified, maybe? Now that we seem
>> to have reduced its scope to expression continuations across newlines.
>>
>> Hopefully it won't be confused with Kernel#callcc.
> 
> Simple is what it is in comparison to something more complex.

Just 1 indent vs arbitrary number of indents depending on operator 
priority/ast nesting. Seems like "simpler" is appropriate.

> All
> indentations are pretty much about line continuation in one way or
> another.

Okay, how about ruby-indent-operator-continuation?

Or ruby-indent-binary-op-continuation. Which would include all binary 
operators and method calls. *shrug* We could also split off the method 
call indentation to a separate option too.

> What is it on its own? I'm not sure.
> 
> Some food for thought:
> 
> Unaligned

That might be a good adjective (if we take it to mean, not aligned to 
the closest parent AST node), but something else to narrow down the 
scope is needed in the name. ruby-operator-unaligned-indent?

ruby-operator-shallow-indent?

> Beginning of line aligned

Beginning of statement, I guess?

> Standard

"Standard" is a point of view. ;-)

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

[Aaron Jensen]

On Tue, Dec 27, 2022 at 10:56 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 27/12/2022 03:47, Aaron Jensen wrote:
> > On Mon, Dec 26, 2022 at 8:28 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >>
> >> On 25/12/2022 02:14, Aaron Jensen wrote:
> >>> (setq ruby-indent-simplified t
> >>
> >> BTW, do you have any opinion on the name? Perhaps something more
> >> semantic would be easier to discover.
> >>
> >> A recent tree-sitter thread brought up sh-indent-after-continuation.
> >> It's not a direct counterpart, though, and the examples only look
> >> remotely similar.
> >>
> >> Call ours ruby-indent-continuations-simplified, maybe? Now that we seem
> >> to have reduced its scope to expression continuations across newlines.
> >>
> >> Hopefully it won't be confused with Kernel#callcc.
> >
> > Simple is what it is in comparison to something more complex.
>
> Just 1 indent vs arbitrary number of indents depending on operator
> priority/ast nesting. Seems like "simpler" is appropriate.

Right, but that was my point. The name doesn't stand on its own. It
only stands relative to some other more complex indentation scheme. If
we can find a name that stands on its own, I think that would be
better.

> > All
> > indentations are pretty much about line continuation in one way or
> > another.
>
> Okay, how about ruby-indent-operator-continuation?
>
> Or ruby-indent-binary-op-continuation. Which would include all binary
> operators and method calls. *shrug* We could also split off the method
> call indentation to a separate option too.

Right, maybe it makes sense to consider one of two directions:

1. A single option to enable this "simple" indentation mode, i.e.
ruby-indent-alignment: line/statement/start/beginning vs. sibling/end
2. Split each different rule into its own option and name them
according to the specific circumstance the rule covers. I still don't
know what the options would be.

That said, when you say method calls, you mean the '.' operator, yes?
I see what you're getting at with this naming and I think it's
probably cohesive enough to be one option per #2 above.

> > What is it on its own? I'm not sure.
> >
> > Some food for thought:
> >
> > Unaligned
>
> That might be a good adjective (if we take it to mean, not aligned to
> the closest parent AST node), but something else to narrow down the
> scope is needed in the name. ruby-operator-unaligned-indent?
>
> ruby-operator-shallow-indent?
>
> > Beginning of line aligned
>
> Beginning of statement, I guess?

Yeah, that would be better than line

> > Standard
>
> "Standard" is a point of view. ;-)

Indeed... there is also https://github.com/testdouble/standard but I
think it's a bit of a land grab to call it standard and I've never
really looked at it.


I put incremental in the last list since I was trying to get at the
fact that the indentation increases by one increment at a time. Is
there something about it being that vs it context-aware? Obviously all
indentation is context aware, so I'm not sure that that's the right
direction.

Aaron

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

[Dmitry Gutov]

On 27/12/2022 18:34, Aaron Jensen wrote:

>>> Simple is what it is in comparison to something more complex.
>>
>> Just 1 indent vs arbitrary number of indents depending on operator
>> priority/ast nesting. Seems like "simpler" is appropriate.
> 
> Right, but that was my point. The name doesn't stand on its own. It
> only stands relative to some other more complex indentation scheme. If
> we can find a name that stands on its own, I think that would be
> better.

That's true.

But it seems we've rejected most of each other's suggestions by now.

>>> All
>>> indentations are pretty much about line continuation in one way or
>>> another.
>>
>> Okay, how about ruby-indent-operator-continuation?
>>
>> Or ruby-indent-binary-op-continuation. Which would include all binary
>> operators and method calls. *shrug* We could also split off the method
>> call indentation to a separate option too.
> 
> Right, maybe it makes sense to consider one of two directions:
> 
> 1. A single option to enable this "simple" indentation mode, i.e.
> ruby-indent-alignment: line/statement/start/beginning vs. sibling/end
> 2. Split each different rule into its own option and name them
> according to the specific circumstance the rule covers. I still don't
> know what the options would be.
> 
> That said, when you say method calls, you mean the '.' operator, yes?
> I see what you're getting at with this naming and I think it's
> probably cohesive enough to be one option per #2 above.

Right. If we consider "." as something distinct, it could use a separate 
option. Or not. But it's trivial to separate.

>> "Standard" is a point of view. ;-)
> 
> Indeed... there is also https://github.com/testdouble/standard but I
> think it's a bit of a land grab to call it standard and I've never
> really looked at it.

Concur.

> I put incremental in the last list since I was trying to get at the
> fact that the indentation increases by one increment at a time.

IDK, there might be different connotations, e.g. it always grows (though 
slowly).

 >  Is
 > there something about it being that vs it context-aware?
> Obviously all
> indentation is context aware, so I'm not sure that that's the right
> direction.

"More" context-aware, one could say. Or less. But that's the same as 
"simpler".

I suppose we could call it structural..? The current behavior, that is. 
As in 
https://github.com/yairchu/awesome-structure-editors#structural-code-editor-projects.

Or here's a step back: looking at how the two other user options I named 
previously were ruby-method-params-indent and ruby-block-indent, the 
latest might as well be called ruby-operator-indent, or 
ruby-operator-indent and ruby-method-call-indent.

I wasn't too crazy about those names originally, but the approach is 
very extensible with styles by adding new symbols as possible values.

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

[Aaron Jensen]

On Tue, Dec 27, 2022 at 6:04 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 27/12/2022 18:34, Aaron Jensen wrote:
>
> >>> Simple is what it is in comparison to something more complex.
> >>
> >> Just 1 indent vs arbitrary number of indents depending on operator
> >> priority/ast nesting. Seems like "simpler" is appropriate.
> >
> > Right, but that was my point. The name doesn't stand on its own. It
> > only stands relative to some other more complex indentation scheme. If
> > we can find a name that stands on its own, I think that would be
> > better.
>
> That's true.
>
> But it seems we've rejected most of each other's suggestions by now.
>
> >>> All
> >>> indentations are pretty much about line continuation in one way or
> >>> another.
> >>
> >> Okay, how about ruby-indent-operator-continuation?
> >>
> >> Or ruby-indent-binary-op-continuation. Which would include all binary
> >> operators and method calls. *shrug* We could also split off the method
> >> call indentation to a separate option too.
> >
> > Right, maybe it makes sense to consider one of two directions:
> >
> > 1. A single option to enable this "simple" indentation mode, i.e.
> > ruby-indent-alignment: line/statement/start/beginning vs. sibling/end
> > 2. Split each different rule into its own option and name them
> > according to the specific circumstance the rule covers. I still don't
> > know what the options would be.
> >
> > That said, when you say method calls, you mean the '.' operator, yes?
> > I see what you're getting at with this naming and I think it's
> > probably cohesive enough to be one option per #2 above.
>
> Right. If we consider "." as something distinct, it could use a separate
> option. Or not. But it's trivial to separate.
>
> >> "Standard" is a point of view. ;-)
> >
> > Indeed... there is also https://github.com/testdouble/standard but I
> > think it's a bit of a land grab to call it standard and I've never
> > really looked at it.
>
> Concur.
>
> > I put incremental in the last list since I was trying to get at the
> > fact that the indentation increases by one increment at a time.
>
> IDK, there might be different connotations, e.g. it always grows (though
> slowly).
>
>  >  Is
>  > there something about it being that vs it context-aware?
> > Obviously all
> > indentation is context aware, so I'm not sure that that's the right
> > direction.
>
> "More" context-aware, one could say. Or less. But that's the same as
> "simpler".
>
> I suppose we could call it structural..? The current behavior, that is.
> As in
> https://github.com/yairchu/awesome-structure-editors#structural-code-editor-projects.
>
> Or here's a step back: looking at how the two other user options I named
> previously were ruby-method-params-indent and ruby-block-indent, the
> latest might as well be called ruby-operator-indent, or
> ruby-operator-indent and ruby-method-call-indent.
>
> I wasn't too crazy about those names originally, but the approach is
> very extensible with styles by adding new symbols as possible values.

This may end up being the right direction. If the values are symbols
you can use things that are relative to one another like "simple".
There could be a benefit to all of these having a "simple" option.
What would it mean if it were nil?
What's the current behavior called?

It may be that if we only intend to support two indentation schemes we
just have default and simplified as you suggested and then we can use
boolean values. I don't know how Emacs-like this is, but what if there
were one variable like `ruby-indent-simple` that could either be `t`
or a list of things to indent simply?

Aaron

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

[Dmitry Gutov]

On 28/12/2022 02:38, Aaron Jensen wrote:

>> Or here's a step back: looking at how the two other user options I named
>> previously were ruby-method-params-indent and ruby-block-indent, the
>> latest might as well be called ruby-operator-indent, or
>> ruby-operator-indent and ruby-method-call-indent.
>>
>> I wasn't too crazy about those names originally, but the approach is
>> very extensible with styles by adding new symbols as possible values.
> 
> This may end up being the right direction. If the values are symbols
> you can use things that are relative to one another like "simple".
> There could be a benefit to all of these having a "simple" option.
> What would it mean if it were nil?
> What's the current behavior called?

For the sake of uniformity, I wanted to start with simple values -- t 
and nil, and explain their meanings in the docstring.

't' would mean the current behavior, and I'd call it "structural", or 
structure-based indentation. Or based on implicit expression grouping.

> It may be that if we only intend to support two indentation schemes we
> just have default and simplified as you suggested and then we can use
> boolean values. I don't know how Emacs-like this is, but what if there
> were one variable like `ruby-indent-simple` that could either be `t`
> or a list of things to indent simply?

That can work too, but what is "simple"? ;-)

Further, I'm not sure if we're going to get more than 2 "things" this 
way (operators and method calls). OTOH, if we have a separate var for 
operators -- ruby-operator-indent -- we could enumerate which operators 
to indent "structurally" after. Or something like that.

Not sure which direction the feature requests will drive this extension 
toward, though. Maybe mostly nowhere, given the previous history. But 
Rubocop's example seems to indicate that there are many different styles 
out there.

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

[Aaron Jensen]

On Tue, Dec 27, 2022 at 8:02 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 28/12/2022 02:38, Aaron Jensen wrote:
>
> >> Or here's a step back: looking at how the two other user options I named
> >> previously were ruby-method-params-indent and ruby-block-indent, the
> >> latest might as well be called ruby-operator-indent, or
> >> ruby-operator-indent and ruby-method-call-indent.
> >>
> >> I wasn't too crazy about those names originally, but the approach is
> >> very extensible with styles by adding new symbols as possible values.
> >
> > This may end up being the right direction. If the values are symbols
> > you can use things that are relative to one another like "simple".
> > There could be a benefit to all of these having a "simple" option.
> > What would it mean if it were nil?
> > What's the current behavior called?
>
> For the sake of uniformity, I wanted to start with simple values -- t
> and nil, and explain their meanings in the docstring.
>
> 't' would mean the current behavior, and I'd call it "structural", or
> structure-based indentation. Or based on implicit expression grouping.

I'd typically not use t and nil on anything but a boolean and the name
would be named after what t represents, but this may be an Emacs idiom
that is OK. If so, and there's no better options (i.e., going against
that idiom is worse than not), then that works for me.


> > It may be that if we only intend to support two indentation schemes we
> > just have default and simplified as you suggested and then we can use
> > boolean values. I don't know how Emacs-like this is, but what if there
> > were one variable like `ruby-indent-simple` that could either be `t`
> > or a list of things to indent simply?
>
> That can work too, but what is "simple"? ;-)
>
> Further, I'm not sure if we're going to get more than 2 "things" this
> way (operators and method calls). OTOH, if we have a separate var for
> operators -- ruby-operator-indent -- we could enumerate which operators
> to indent "structurally" after. Or something like that.
>
> Not sure which direction the feature requests will drive this extension
> toward, though. Maybe mostly nowhere, given the previous history. But
> Rubocop's example seems to indicate that there are many different styles
> out there.

Yeah, hard to say, and it may eventually become less important if
ruby-ts comes about and has enough options to satisfy folks with
different ideas about indentation.

Aaron

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

[Dmitry Gutov]

On 28/12/2022 05:47, Aaron Jensen wrote:
> On Tue, Dec 27, 2022 at 8:02 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> On 28/12/2022 02:38, Aaron Jensen wrote:
>>
>>>> Or here's a step back: looking at how the two other user options I named
>>>> previously were ruby-method-params-indent and ruby-block-indent, the
>>>> latest might as well be called ruby-operator-indent, or
>>>> ruby-operator-indent and ruby-method-call-indent.
>>>>
>>>> I wasn't too crazy about those names originally, but the approach is
>>>> very extensible with styles by adding new symbols as possible values.
>>>
>>> This may end up being the right direction. If the values are symbols
>>> you can use things that are relative to one another like "simple".
>>> There could be a benefit to all of these having a "simple" option.
>>> What would it mean if it were nil?
>>> What's the current behavior called?
>>
>> For the sake of uniformity, I wanted to start with simple values -- t
>> and nil, and explain their meanings in the docstring.
>>
>> 't' would mean the current behavior, and I'd call it "structural", or
>> structure-based indentation. Or based on implicit expression grouping.
> 
> I'd typically not use t and nil on anything but a boolean and the name
> would be named after what t represents, but this may be an Emacs idiom
> that is OK. If so, and there's no better options (i.e., going against
> that idiom is worse than not), then that works for me.

I guess that particular trend started with ruby-method-params-indent, 
where I haven't managed to choose better names for the var, or the values.

>>> It may be that if we only intend to support two indentation schemes we
>>> just have default and simplified as you suggested and then we can use
>>> boolean values. I don't know how Emacs-like this is, but what if there
>>> were one variable like `ruby-indent-simple` that could either be `t`
>>> or a list of things to indent simply?
>>
>> That can work too, but what is "simple"? ;-)
>>
>> Further, I'm not sure if we're going to get more than 2 "things" this
>> way (operators and method calls). OTOH, if we have a separate var for
>> operators -- ruby-operator-indent -- we could enumerate which operators
>> to indent "structurally" after. Or something like that.
>>
>> Not sure which direction the feature requests will drive this extension
>> toward, though. Maybe mostly nowhere, given the previous history. But
>> Rubocop's example seems to indicate that there are many different styles
>> out there.
> 
> Yeah, hard to say, and it may eventually become less important if
> ruby-ts comes about and has enough options to satisfy folks with
> different ideas about indentation.

ruby-ts-mode might have extra flexibility down the line, but at the 
start I want to have it obey the same indentation options as ruby-mode.

BTW, you can check out its progress at 
https://github.com/pedz/ruby-ts-mode/.

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

On 28/12/2022 14:47, Dmitry Gutov wrote:
> I guess that particular trend started with ruby-method-params-indent, 
> where I haven't managed to choose better names for the var, or the values.

Semantics aside (I suppose we could go back and revise the naming a 
little later), could you test this new revision of the patch?

I think I got the implementation simple enough now.

The number of options has grown, though:

(setq ruby-after-operator-indent nil
       ruby-block-indent nil
       ruby-method-call-indent nil
       ruby-parenless-call-arguments-indent nil)

[-- Attachment #2: ruby-simplified-indent-v6.diff --]
[-- Type: text/x-patch, Size: 10761 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index a4aa61905e4..021dda1a3e1 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -285,6 +285,92 @@ ruby-method-params-indent
   :safe (lambda (val) (or (memq val '(t nil)) (numberp val)))
   :version "29.1")
 
+(defcustom ruby-block-indent t
+  "Non-nil to align the body of a block to the statement's start.
+
+The body and the closer will be aligned to the column where the
+statement containing the block starts. Example:
+
+  foo.bar
+    .each do
+    baz
+  end
+
+If nil, it will be aligned instead to the beginning of the line
+containing the block's opener:
+
+  foo.bar
+    .each do
+      baz
+    end
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
+(defcustom ruby-after-operator-indent t
+  "Non-nil to use structural indentation after binary operators.
+
+The code will be aligned to the implicit parent expression,
+according to the operator precedence:
+
+  qux = 4 + 5 *
+            6 +
+        7
+
+Set it to nil to align to the beginning of the statement:
+
+  qux = 4 + 5 *
+    6 +
+    7
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
+(defcustom ruby-method-call-indent t
+  "Non-nil to use the structural indentation algorithm.
+
+The method call will be aligned to the implicit parent
+expression, according to the operator precedence:
+
+  foo = subject
+          .update(
+            1
+          )
+
+Set it to nil to align to the beginning of the statement:
+
+  foo = subject
+    .update(
+      1
+    )
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
+(defcustom ruby-parenless-call-arguments-indent t
+  "Non-nil to align arguments in a parenless call vertically.
+
+Example:
+
+  qux :+,
+      bar,
+      :[]=,
+      bar
+
+Set it to nil to align to the beginning of the statement:
+
+  qux :+,
+    bar,
+    :[]=,
+    bar
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp)
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -416,6 +502,7 @@ ruby-smie-grammar
      '((right "=")
        (right "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^="
               "<<=" ">>=" "&&=" "||=")
+       (right "?")
        (nonassoc ".." "...")
        (left "&&" "||")
        (nonassoc "<=>")
@@ -608,10 +695,10 @@ ruby-smie--backward-token
           "def=")
          (t tok)))))))
 
-(defun ruby-smie--indent-to-stmt ()
+(defun ruby-smie--indent-to-stmt (&optional offset)
   (save-excursion
     (smie-backward-sexp ";")
-    (cons 'column (smie-indent-virtual))))
+    (cons 'column (+ (smie-indent-virtual) (or offset 0)))))
 
 (defun ruby-smie--indent-to-stmt-p (keyword)
   (or (eq t ruby-align-to-stmt-keywords)
@@ -642,7 +729,9 @@ ruby-smie-rules
               (forward-comment -1)
               (not (eq (preceding-char) ?:))))
        ;; Curly block opener.
-       (ruby-smie--indent-to-stmt))
+       (if ruby-block-indent
+           (ruby-smie--indent-to-stmt)
+         (cons 'column (current-indentation))))
       ((smie-rule-hanging-p)
        ;; Treat purely syntactic block-constructs as being part of their parent,
        ;; when the opening token is hanging and the parent is not an
@@ -677,13 +766,20 @@ ruby-smie-rules
        (unless (or (eolp) (forward-comment 1))
          (cons 'column (current-column)))))
     ('(:before . " @ ")
-     (if (or (eq ruby-method-params-indent t)
-             (not (smie-rule-parent-p "def" "def=")))
-         (save-excursion
-           (skip-chars-forward " \t")
-           (cons 'column (current-column)))
-       (smie-rule-parent (or ruby-method-params-indent 0))))
-    ('(:before . "do") (ruby-smie--indent-to-stmt))
+     (cond
+      ((and (not ruby-parenless-call-arguments-indent)
+            (not (smie-rule-parent-p "def" "def=")))
+       (ruby-smie--indent-to-stmt ruby-indent-level))
+      ((or (eq ruby-method-params-indent t)
+           (not (smie-rule-parent-p "def" "def=")))
+       (save-excursion
+         (skip-chars-forward " \t")
+         (cons 'column (current-column))))
+      (t (smie-rule-parent (or ruby-method-params-indent 0)))))
+    ('(:before . "do")
+     (if ruby-block-indent
+         (ruby-smie--indent-to-stmt)
+       (cons 'column (current-indentation))))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
          (when ruby-align-chained-calls
@@ -696,8 +792,10 @@ ruby-smie-rules
                    (not (smie-rule-bolp)))))
            (cons 'column (current-column)))
        (smie-backward-sexp ".")
-       (cons 'column (+ (current-column)
-                        ruby-indent-level))))
+       (if ruby-method-call-indent
+           (cons 'column (+ (current-column)
+                            ruby-indent-level))
+         (ruby-smie--indent-to-stmt ruby-indent-level))))
     (`(:before . ,(or "else" "then" "elsif" "rescue" "ensure"))
      (smie-rule-parent))
     (`(:before . ,(or "when" "in"))
@@ -708,16 +806,22 @@ ruby-smie-rules
                      "<=>" ">" "<" ">=" "<=" "==" "===" "!=" "<<" ">>"
                      "+=" "-=" "*=" "/=" "%=" "**=" "&=" "|=" "^=" "|"
                      "<<=" ">>=" "&&=" "||=" "and" "or"))
-     (and (smie-rule-parent-p ";" nil)
-          (smie-indent--hanging-p)
-          ruby-indent-level))
+     (cond
+      ((not ruby-after-operator-indent)
+       (ruby-smie--indent-to-stmt ruby-indent-level))
+      ((and (smie-rule-parent-p ";" nil)
+            (smie-indent--hanging-p))
+       ruby-indent-level)))
     (`(:before . "=")
      (save-excursion
       (and (smie-rule-parent-p " @ ")
            (goto-char (nth 1 (smie-indent--parent)))
            (smie-rule-prev-p "def=")
            (cons 'column (+ (current-column) ruby-indent-level -3)))))
-    (`(:after . ,(or "?" ":")) ruby-indent-level)
+    (`(:after . ,(or "?" ":"))
+     (if ruby-after-operator-indent
+         ruby-indent-level
+       (ruby-smie--indent-to-stmt ruby-indent-level)))
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
        (if (ruby-smie--indent-to-stmt-p token)
@@ -725,7 +829,10 @@ ruby-smie-rules
          (cons 'column (current-column)))))
     ('(:before . "iuwu-mod")
      (smie-rule-parent ruby-indent-level))
-    ))
+    (`(:before . ",")
+     (and (not ruby-parenless-call-arguments-indent)
+          (smie-rule-parent-p " @ ")
+          (ruby-smie--indent-to-stmt ruby-indent-level)))))
 
 (defun ruby--at-indentation-p (&optional point)
   (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-after-operator-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-after-operator-indent.rb
new file mode 100644
index 00000000000..25cd8736f97
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-after-operator-indent.rb
@@ -0,0 +1,29 @@
+4 +
+  5 +
+  6 +
+  7
+
+qux = 4 + 5 *
+  6 +
+  7
+
+foo = obj.bar { |m| tee(m) } +
+  obj.qux { |m| hum(m) }
+
+foo.
+  bar
+  .baz
+
+qux = foo.fee ?
+  bar :
+  tee
+
+# Endless methods.
+class Bar
+  def foo(abc) = bar +
+    baz
+end
+
+# Local Variables:
+# ruby-after-operator-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb
new file mode 100644
index 00000000000..32882814b7e
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-block-indent.rb
@@ -0,0 +1,33 @@
+foo
+  .asdasd
+  .proc do |**args|
+    p(**args)
+  end
+
+foo
+  .asdasd
+  .proc { |**args|
+    p(**args)
+  }
+
+bar.foo do
+  bar
+end
+
+bar.foo(tee) do
+  bar
+end
+
+bar.foo(tee) {
+  bar
+}
+
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
+# Local Variables:
+# ruby-block-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-method-call-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-method-call-indent.rb
new file mode 100644
index 00000000000..1a8285ee919
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-method-call-indent.rb
@@ -0,0 +1,15 @@
+foo2 =
+  subject.
+  update(
+    2
+  )
+
+foo3 =
+  subject
+  .update(
+    2
+  )
+
+# Local Variables:
+# ruby-method-call-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-parenless-call-arguments-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-parenless-call-arguments-indent.rb
new file mode 100644
index 00000000000..58e08810c4c
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-parenless-call-arguments-indent.rb
@@ -0,0 +1,23 @@
+method arg1,
+  method2 arg2,
+  arg3, [
+    arg4,
+    arg5
+  ]
+
+zzz = method (a + b),
+  c, :d => :e,
+  f: g
+
+return render json: {
+    errors: { base: [message] },
+    copying: copying
+  },
+  status: 400
+
+foo(a,
+    b)
+
+# Local Variables:
+# ruby-parenless-call-arguments-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index 6a69d9db78a..bfae948b259 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -226,6 +226,7 @@ def begin
 
 foo.
   bar
+  .baz
 
 # https://github.com/rails/rails/blob/17f5d8e062909f1fcae25351834d8e89967b645e/activesupport/lib/active_support/time_with_zone.rb#L206
 foo # comment intended to confuse the tokenizer
@@ -380,6 +381,18 @@ def bar
   i + 1
 end
 
+m1 = foo
+       .asdasd
+       .proc do |**args|
+  p(**args)
+end
+
+m2 = foo
+       .asdasd
+       .proc { |**args|
+  p(**args)
+}
+
 bar.foo do
   bar
 end
@@ -398,6 +411,12 @@ def bar
   end
 end
 
+x.foo do
+  foo
+end.bar do
+  bar
+end
+
 foo |
   bar
 
@@ -540,5 +559,9 @@ def baz.full_name = "#{bar} 3"
 end
 
 # Local Variables:
+# ruby-after-operator-indent: t
+# ruby-block-indent: t
+# ruby-method-call-indent: t
 # ruby-method-params-indent: t
+# ruby-parenless-call-arguments-indent: t
 # End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 560f780285a..5c81cc31cc1 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -956,7 +956,11 @@ ruby-deftest-indent
          (kill-buffer buf)))))
 
 (ruby-deftest-indent "ruby.rb")
+(ruby-deftest-indent "ruby-after-operator-indent.rb")
+(ruby-deftest-indent "ruby-block-indent.rb")
+(ruby-deftest-indent "ruby-method-call-indent.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
+(ruby-deftest-indent "ruby-parenless-call-arguments-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer

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

[Aaron Jensen]

On Wed, Dec 28, 2022 at 4:24 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 28/12/2022 14:47, Dmitry Gutov wrote:
> > I guess that particular trend started with ruby-method-params-indent,
> > where I haven't managed to choose better names for the var, or the values.
>
> Semantics aside (I suppose we could go back and revise the naming a
> little later), could you test this new revision of the patch?
>
> I think I got the implementation simple enough now.
>
> The number of options has grown, though:
>
> (setq ruby-after-operator-indent nil
>        ruby-block-indent nil
>        ruby-method-call-indent nil
>        ruby-parenless-call-arguments-indent nil)

I'll give it a shot some more, but these appear problematic:

fixture(
EntityProjection::Fixtures::Projection,
projection,
deleted
) do |projection|
  projection.assert_attributes_copied([
    { :document_id => :id }
                                      ])
end

Should be:

fixture(
  EntityProjection::Fixtures::Projection,
  projection,
  deleted
) do |projection|
  projection.assert_attributes_copied([
    { :document_id => :id }
  ])
end

Aaron

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

[Dmitry Gutov]

Hi Aaron,

On 30/12/2022 00:59, Aaron Jensen wrote:
> On Wed, Dec 28, 2022 at 4:24 PM Dmitry Gutov<dgutov@yandex.ru>  wrote:
>> On 28/12/2022 14:47, Dmitry Gutov wrote:
>>> I guess that particular trend started with ruby-method-params-indent,
>>> where I haven't managed to choose better names for the var, or the values.
>> Semantics aside (I suppose we could go back and revise the naming a
>> little later), could you test this new revision of the patch?
>>
>> I think I got the implementation simple enough now.
>>
>> The number of options has grown, though:
>>
>> (setq ruby-after-operator-indent nil
>>         ruby-block-indent nil
>>         ruby-method-call-indent nil
>>         ruby-parenless-call-arguments-indent nil)
> I'll give it a shot some more, but these appear problematic:
> 
> fixture(
> EntityProjection::Fixtures::Projection,
> projection,
> deleted
> ) do|projection|
>    projection.assert_attributes_copied([
>      { :document_id => :id }
>                                        ])
> end
> 
> Should be:
> 
> fixture(
>    EntityProjection::Fixtures::Projection,
>    projection,
>    deleted
> ) do|projection|
>    projection.assert_attributes_copied([
>      { :document_id => :id }
>    ])
> end

This example is for https://debbugs.gnu.org/60321, I think. Which we 
split off and postponed a little.

The last patch was the latest revision of the changes for 
https://debbugs.gnu.org/60186.

As long as this example is unchanged (indented the same as the current 
version of ruby-mode), it's good enough. It looks like this on my 
machine, though:

   fixture(
     EntityProjection::Fixtures::Projection,
     projection,
     deleted
   ) do |projection|
     projection.assert_attributes_copied([
                                           { :document_id => :id }
                                         ])
   end

Could you please give the v6 patch a good run soon-ish: we have a good 
chance of getting it into Emacs 29 (together with ruby-ts-mode, which 
should reuse some or most of the options), as I've just found out.

The deadline for checking all this in is pretty near, alas: 1-2 days.

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

[Aaron Jensen]

On Fri, Dec 30, 2022 at 10:02 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Hi Aaron,
>
> On 30/12/2022 00:59, Aaron Jensen wrote:
> > On Wed, Dec 28, 2022 at 4:24 PM Dmitry Gutov<dgutov@yandex.ru>  wrote:
> >> On 28/12/2022 14:47, Dmitry Gutov wrote:
> >>> I guess that particular trend started with ruby-method-params-indent,
> >>> where I haven't managed to choose better names for the var, or the values.
> >> Semantics aside (I suppose we could go back and revise the naming a
> >> little later), could you test this new revision of the patch?
> >>
> >> I think I got the implementation simple enough now.
> >>
> >> The number of options has grown, though:
> >>
> >> (setq ruby-after-operator-indent nil
> >>         ruby-block-indent nil
> >>         ruby-method-call-indent nil
> >>         ruby-parenless-call-arguments-indent nil)
> > I'll give it a shot some more, but these appear problematic:
> >
> > fixture(
> > EntityProjection::Fixtures::Projection,
> > projection,
> > deleted
> > ) do|projection|
> >    projection.assert_attributes_copied([
> >      { :document_id => :id }
> >                                        ])
> > end
> >
> > Should be:
> >
> > fixture(
> >    EntityProjection::Fixtures::Projection,
> >    projection,
> >    deleted
> > ) do|projection|
> >    projection.assert_attributes_copied([
> >      { :document_id => :id }
> >    ])
> > end
>
> This example is for https://debbugs.gnu.org/60321, I think. Which we
> split off and postponed a little.
>
> The last patch was the latest revision of the changes for
> https://debbugs.gnu.org/60186.

My mistake, it seems to work for these things aside from the method
params in the example below. I can't reproduce that in emacs -Q, only
with my own config, so I will have to see if I can figure out what the
difference is.


> As long as this example is unchanged (indented the same as the current
> version of ruby-mode), it's good enough. It looks like this on my
> machine, though:
>
>    fixture(
>      EntityProjection::Fixtures::Projection,
>      projection,
>      deleted
>    ) do |projection|
>      projection.assert_attributes_copied([
>                                            { :document_id => :id }
>                                          ])
>    end
>
> Could you please give the v6 patch a good run soon-ish: we have a good
> chance of getting it into Emacs 29 (together with ruby-ts-mode, which
> should reuse some or most of the options), as I've just found out.
>
> The deadline for checking all this in is pretty near, alas: 1-2 days.

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

[Aaron Jensen]

On Fri, Dec 30, 2022 at 1:00 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Fri, Dec 30, 2022 at 10:02 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >
> > Hi Aaron,
> >
> > On 30/12/2022 00:59, Aaron Jensen wrote:
> > > On Wed, Dec 28, 2022 at 4:24 PM Dmitry Gutov<dgutov@yandex.ru>  wrote:
> > >> On 28/12/2022 14:47, Dmitry Gutov wrote:
> > >>> I guess that particular trend started with ruby-method-params-indent,
> > >>> where I haven't managed to choose better names for the var, or the values.
> > >> Semantics aside (I suppose we could go back and revise the naming a
> > >> little later), could you test this new revision of the patch?
> > >>
> > >> I think I got the implementation simple enough now.
> > >>
> > >> The number of options has grown, though:
> > >>
> > >> (setq ruby-after-operator-indent nil
> > >>         ruby-block-indent nil
> > >>         ruby-method-call-indent nil
> > >>         ruby-parenless-call-arguments-indent nil)
> > > I'll give it a shot some more, but these appear problematic:
> > >
> > > fixture(
> > > EntityProjection::Fixtures::Projection,
> > > projection,
> > > deleted
> > > ) do|projection|
> > >    projection.assert_attributes_copied([
> > >      { :document_id => :id }
> > >                                        ])
> > > end
> > >
> > > Should be:
> > >
> > > fixture(
> > >    EntityProjection::Fixtures::Projection,
> > >    projection,
> > >    deleted
> > > ) do|projection|
> > >    projection.assert_attributes_copied([
> > >      { :document_id => :id }
> > >    ])
> > > end
> >
> > This example is for https://debbugs.gnu.org/60321, I think. Which we
> > split off and postponed a little.
> >
> > The last patch was the latest revision of the changes for
> > https://debbugs.gnu.org/60186.
>
> My mistake, it seems to work for these things aside from the method
> params in the example below. I can't reproduce that in emacs -Q, only
> with my own config, so I will have to see if I can figure out what the
> difference is.


I can't reproduce this anymore, I think it had to do w/ dtrt-indent
doing something odd. I had things in a state where some buffers did it
and not others and I restarted Emacs before checking the settings. In
any case, I think it's fine. I don't have any concerns with the patch
as-is.

Thanks,

Aaron

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

[Dmitry Gutov]

Version: 29.1

On 30/12/2022 20:16, Aaron Jensen wrote:
> I can't reproduce this anymore, I think it had to do w/ dtrt-indent
> doing something odd. I had things in a state where some buffers did it
> and not others and I restarted Emacs before checking the settings. In
> any case, I think it's fine. I don't have any concerns with the patch
> as-is.

Thanks for testing, this is now in the release branch.

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

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

 Thank you.


Aaron

On Fri, Dec 30 2022 at 5:07 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Version: 29.1
>
> On 30/12/2022 20:16, Aaron Jensen wrote:
>
> I can't reproduce this anymore, I think it had to do w/ dtrt-indent doing
> something odd. I had things in a state where some buffers did it and not
> others and I restarted Emacs before checking the settings. In any case, I
> think it's fine. I don't have any concerns with the patch as-is.
>
> Thanks for testing, this is now in the release branch.
>

[-- Attachment #2: Type: text/html, Size: 1094 bytes --]

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

[Dmitry Gutov]

Hi Aaron,

Now might be good to test how ruby-ts-mode works with all ruby-mode's 
indentation variables, too. If you have the time.

It should work support all of them now, but I might have missed some 
edge cases.

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

[Aaron Jensen]

On Sat, Jan 21, 2023 at 10:02 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Hi Aaron,
>
> Now might be good to test how ruby-ts-mode works with all ruby-mode's
> indentation variables, too. If you have the time.
>
> It should work support all of them now, but I might have missed some
> edge cases.

Hi Dmitry,

I played around with it briefly and nothing stood out. It looks pretty
good. I'll try switching to it for daily use and report back if I find
anything. Thank you.

Aaron

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

[Aaron Jensen]

[-- Attachment #1.1: Type: text/plain, Size: 2503 bytes --]

I made an attempt at this (attached). It introduces a new variable:

ruby-bracketed-args-indent

It is set to t by default and the behavior will be as it was before this
patch.

If it is something other than t, it will cause enable indentation like this:

update({
  key => value,
  other_key:
}, {
  key => value,
  other_key:
})

update([
  1,
  2
], [
  3,
  4
])

It does not handle cases such as:

some_method({
  key: :value
},
other_argument)

It will indent other_argument to be aligned with the (. This could be
elaborated further, but I contend that if people are formatting their code
this way that they likely have rather idiosyncratic formatting requirements
and they would be best left to do what they want manually.

Thanks,


Aaron


On Mon, Dec 26, 2022 at 8:38 PM, Aaron Jensen <aaronjensen@gmail.com> wrote:

> On Mon, Dec 26, 2022 at 8:16 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Vim's choice looks saner to my eye. Probably comes down to the choice of
> indentation algorithm, though.
>
> Agreed, though it's hard to pick which is more sane when all the options
> start with insanity.
>
> If I had to type it as above, I would probably indent it like this:
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> But I can't imagine that would be easy to implement at all, so I wouldn't
> bother.
>
> The indentation logic itself might be not that difficult to write, but the
> fact that the expression will have to be reindented as soon as the method
> call grows a second argument (after the user types the comma?), makes it a
> hard sell usability-wise.
>
> Right, I think that's just more of the same thing... We are looking at
> ways of writing code that are out of the realm of reason. It's a challenge
> to define behavior when the behavior could very well be undefined. But, if
> we must define it, then what are our guiding principles? Not having to
> reindent preceding lines when adding a new line may be a very reasonable
> one. In that case, the only two options I could think of would be:
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> or
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> The difference being if we decide to dedent upon the last closing
> indent-requiring-token or the first.
>
> I think a reasonable rule of thumb for a human might be: "If you open N
> indents on one line, you must close N indents on one line". Any time you
> stray away from this, behavior becomes... not ideal.
>
> Aaron
>

[-- Attachment #1.2: Type: text/html, Size: 4522 bytes --]

[-- Attachment #2: 0001-Add-ruby-bracketed-argument-indentation-option.patch --]
[-- Type: application/octet-stream, Size: 3191 bytes --]

From 32b1ce281783b98ad64b1d99ae146a23753a5a80 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 31 Aug 2024 19:31:20 -0400
Subject: [PATCH] Add ruby bracketed argument indentation option

* lisp/progmodes/ruby-mode.el (ruby-bracketed-args-indent),
(ruby-smie-rules): New option
* test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb:
* test/lisp/progmodes/ruby-mode-tests.el
  ("ruby-parenless-call-arguments-indent.rb"): New test case
---
 lisp/progmodes/ruby-mode.el                   | 23 +++++++++++++
 .../ruby-bracketed-args-indent.rb             | 33 +++++++++++++++++++
 test/lisp/progmodes/ruby-mode-tests.el        |  1 +
 3 files changed, 57 insertions(+)
 create mode 100644 test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 3bcfa9ee7df..85cdce39937 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -472,6 +472,26 @@ ruby-parenless-call-arguments-indent
   :safe 'booleanp
   :version "29.1")
 
+(defcustom ruby-bracketed-args-indent t
+  "Non-nil to align the contents of bracketed arguments with the brackets.
+
+Example:
+
+  qux({
+       foo => bar
+     })
+
+Set it to nil to align to the beginning of the statement:
+
+  qux({
+    foo => bar
+  })
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp
+  :version "31.1")
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -826,6 +846,9 @@ ruby-smie-rules
       ))
     (`(:before . ,(or "(" "[" "{"))
      (cond
+      ((and (not (eq ruby-bracketed-args-indent t))
+            (smie-rule-prev-p "," "(" "["))
+       (cons 'column (current-indentation)))
       ((and (equal token "{")
             (not (smie-rule-prev-p "(" "{" "[" "," "=>" "=" "return" ";" "do"))
             (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
new file mode 100644
index 00000000000..7d3df3463ff
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
@@ -0,0 +1,33 @@
+update({
+  key => value,
+  other_key:
+}, {
+  key => value,
+  other_key:
+})
+
+update([
+  1,
+  2
+], [
+  3,
+  4
+])
+
+update([{
+  key: "value"
+}, {
+  key: "value"
+}])
+
+update(arg1, {
+  foo: "bar"
+}, [
+  1,
+  2
+], arg2)
+
+# Local Variables:
+# ruby-bracketed-args-indent: nil
+# ruby-method-params-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 2b8506a7adc..c9cde791baa 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -992,6 +992,7 @@ "ruby-block-indent.rb"
 (ruby-deftest-indent "ruby-method-call-indent.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
 (ruby-deftest-indent "ruby-parenless-call-arguments-indent.rb")
+(ruby-deftest-indent "ruby-bracketed-args-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer
-- 
2.42.1

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

[Aaron Jensen]

[-- Attachment #1.1: Type: text/plain, Size: 2732 bytes --]

Updated patch with more precise variables in the new test.


Aaron


On Sat, Aug 31, 2024 at 7:41 PM, Aaron Jensen <aaronjensen@gmail.com> wrote:

> I made an attempt at this (attached). It introduces a new variable:
>
> ruby-bracketed-args-indent
>
> It is set to t by default and the behavior will be as it was before this
> patch.
>
> If it is something other than t, it will cause enable indentation like
> this:
>
> update({
>   key => value,
>   other_key:
> }, {
>   key => value,
>   other_key:
> })
>
> update([
>   1,
>   2
> ], [
>   3,
>   4
> ])
>
> It does not handle cases such as:
>
> some_method({
>   key: :value
> },
> other_argument)
>
> It will indent other_argument to be aligned with the (. This could be
> elaborated further, but I contend that if people are formatting their code
> this way that they likely have rather idiosyncratic formatting requirements
> and they would be best left to do what they want manually.
>
> Thanks,
>
>
> Aaron
>
>
> On Mon, Dec 26, 2022 at 8:38 PM, Aaron Jensen <aaronjensen@gmail.com>
> wrote:
>
> On Mon, Dec 26, 2022 at 8:16 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Vim's choice looks saner to my eye. Probably comes down to the choice of
> indentation algorithm, though.
>
> Agreed, though it's hard to pick which is more sane when all the options
> start with insanity.
>
> If I had to type it as above, I would probably indent it like this:
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> But I can't imagine that would be easy to implement at all, so I wouldn't
> bother.
>
> The indentation logic itself might be not that difficult to write, but the
> fact that the expression will have to be reindented as soon as the method
> call grows a second argument (after the user types the comma?), makes it a
> hard sell usability-wise.
>
> Right, I think that's just more of the same thing... We are looking at
> ways of writing code that are out of the realm of reason. It's a challenge
> to define behavior when the behavior could very well be undefined. But, if
> we must define it, then what are our guiding principles? Not having to
> reindent preceding lines when adding a new line may be a very reasonable
> one. In that case, the only two options I could think of would be:
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> or
>
> and_in_a_method_call({
> no: :difference
> },
> foo,
> bar)
>
> The difference being if we decide to dedent upon the last closing
> indent-requiring-token or the first.
>
> I think a reasonable rule of thumb for a human might be: "If you open N
> indents on one line, you must close N indents on one line". Any time you
> stray away from this, behavior becomes... not ideal.
>
> Aaron
>
>

[-- Attachment #1.2: Type: text/html, Size: 6598 bytes --]

[-- Attachment #2: 0001-Add-ruby-bracketed-argument-indentation-option.patch --]
[-- Type: application/octet-stream, Size: 3157 bytes --]

From 1b5826dda4ff8d014d37f90b22569e376db85296 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 31 Aug 2024 19:31:20 -0400
Subject: [PATCH] Add ruby bracketed argument indentation option

* lisp/progmodes/ruby-mode.el (ruby-bracketed-args-indent),
(ruby-smie-rules): New option
* test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb:
* test/lisp/progmodes/ruby-mode-tests.el
  ("ruby-parenless-call-arguments-indent.rb"): New test case
---
 lisp/progmodes/ruby-mode.el                   | 23 +++++++++++++
 .../ruby-bracketed-args-indent.rb             | 32 +++++++++++++++++++
 test/lisp/progmodes/ruby-mode-tests.el        |  1 +
 3 files changed, 56 insertions(+)
 create mode 100644 test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 3bcfa9ee7df..85cdce39937 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -472,6 +472,26 @@ ruby-parenless-call-arguments-indent
   :safe 'booleanp
   :version "29.1")
 
+(defcustom ruby-bracketed-args-indent t
+  "Non-nil to align the contents of bracketed arguments with the brackets.
+
+Example:
+
+  qux({
+       foo => bar
+     })
+
+Set it to nil to align to the beginning of the statement:
+
+  qux({
+    foo => bar
+  })
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp
+  :version "31.1")
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -826,6 +846,9 @@ ruby-smie-rules
       ))
     (`(:before . ,(or "(" "[" "{"))
      (cond
+      ((and (not (eq ruby-bracketed-args-indent t))
+            (smie-rule-prev-p "," "(" "["))
+       (cons 'column (current-indentation)))
       ((and (equal token "{")
             (not (smie-rule-prev-p "(" "{" "[" "," "=>" "=" "return" ";" "do"))
             (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
new file mode 100644
index 00000000000..ac7a73463bf
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
@@ -0,0 +1,32 @@
+update({
+  key => value,
+  other_key:
+}, {
+  key => value,
+  other_key:
+})
+
+update([
+  1,
+  2
+], [
+  3,
+  4
+])
+
+update([{
+  key: "value"
+}, {
+  key: "value"
+}])
+
+update(arg1, {
+  foo: "bar"
+}, [
+  1,
+  2
+], arg2)
+
+# Local Variables:
+# ruby-bracketed-args-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 2b8506a7adc..c9cde791baa 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -992,6 +992,7 @@ "ruby-block-indent.rb"
 (ruby-deftest-indent "ruby-method-call-indent.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
 (ruby-deftest-indent "ruby-parenless-call-arguments-indent.rb")
+(ruby-deftest-indent "ruby-bracketed-args-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer
-- 
2.42.1

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

[Dmitry Gutov]

Hi Aaron!

On 01/09/2024 03:54, Aaron Jensen wrote:
> Updated patch with more precise variables in the new test.

Thanks for taking the initiative.

Here's an example which seems to get worse with the new variable set to nil:

def foo
   foo.update(
{
   key => value,
   other_key: foo
}
   )
end

I'd like to flip the default value (now or in Emacs 31), so it would be 
great to deal with examples like this.

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

[Aaron Jensen]

[-- Attachment #1.1: Type: text/plain, Size: 1005 bytes --]

Hi Dmitry,

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.

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.

Thanks,


Aaron


On Sun, Sep 01, 2024 at 12:36 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> Hi Aaron!
>
> On 01/09/2024 03:54, Aaron Jensen wrote:
>
> Updated patch with more precise variables in the new test.
>
> Thanks for taking the initiative.
>
> Here's an example which seems to get worse with the new variable set to
> nil:
>
> def foo
> foo.update(
> {
> key => value,
> other_key: foo
> }
> )
> end
>
> I'd like to flip the default value (now or in Emacs 31), so it would be
> great to deal with examples like this.
>

[-- Attachment #1.2: Type: text/html, Size: 2017 bytes --]

[-- Attachment #2: 0001-Add-ruby-bracketed-argument-indentation-option.patch --]
[-- Type: application/octet-stream, Size: 3194 bytes --]

From 1c204a8834c8e3e590fb851433d86a17bcffd9a4 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 31 Aug 2024 19:31:20 -0400
Subject: [PATCH] Add ruby bracketed argument indentation option

* lisp/progmodes/ruby-mode.el (ruby-bracketed-args-indent),
(ruby-smie-rules): New option
* test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb:
* test/lisp/progmodes/ruby-mode-tests.el
  ("ruby-parenless-call-arguments-indent.rb"): New test case
---
 lisp/progmodes/ruby-mode.el                   | 24 ++++++++++++++
 .../ruby-bracketed-args-indent.rb             | 32 +++++++++++++++++++
 test/lisp/progmodes/ruby-mode-tests.el        |  1 +
 3 files changed, 57 insertions(+)
 create mode 100644 test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 3bcfa9ee7df..741b5167132 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -472,6 +472,26 @@ ruby-parenless-call-arguments-indent
   :safe 'booleanp
   :version "29.1")
 
+(defcustom ruby-bracketed-args-indent t
+  "Non-nil to align the contents of bracketed arguments with the brackets.
+
+Example:
+
+  qux({
+       foo => bar
+     })
+
+Set it to nil to align to the beginning of the statement:
+
+  qux({
+    foo => bar
+  })
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp
+  :version "31.1")
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -826,6 +846,10 @@ ruby-smie-rules
       ))
     (`(:before . ,(or "(" "[" "{"))
      (cond
+      ((and (not (eq ruby-bracketed-args-indent t))
+            (smie-rule-prev-p "," "(" "[")
+            (smie-rule-hanging-p))
+       (cons 'column (current-indentation)))
       ((and (equal token "{")
             (not (smie-rule-prev-p "(" "{" "[" "," "=>" "=" "return" ";" "do"))
             (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
new file mode 100644
index 00000000000..ac7a73463bf
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
@@ -0,0 +1,32 @@
+update({
+  key => value,
+  other_key:
+}, {
+  key => value,
+  other_key:
+})
+
+update([
+  1,
+  2
+], [
+  3,
+  4
+])
+
+update([{
+  key: "value"
+}, {
+  key: "value"
+}])
+
+update(arg1, {
+  foo: "bar"
+}, [
+  1,
+  2
+], arg2)
+
+# Local Variables:
+# ruby-bracketed-args-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 2b8506a7adc..c9cde791baa 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -992,6 +992,7 @@ "ruby-block-indent.rb"
 (ruby-deftest-indent "ruby-method-call-indent.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
 (ruby-deftest-indent "ruby-parenless-call-arguments-indent.rb")
+(ruby-deftest-indent "ruby-bracketed-args-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer
-- 
2.42.1

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

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


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.

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

* Let's change the first example to this, for less ambiguity?

   foo
     .update({
       key => value,
       other_key:
     }, {
       key => value,
       other_key:
     })


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

[-- Attachment #2: ruby-ts-bracketed-argument-indentation-option.diff --]
[-- Type: text/x-patch, Size: 1499 bytes --]

diff --git a/lisp/progmodes/ruby-ts-mode.el b/lisp/progmodes/ruby-ts-mode.el
index 5f4e11e0b4c..adcdf15c7ad 100644
--- a/lisp/progmodes/ruby-ts-mode.el
+++ b/lisp/progmodes/ruby-ts-mode.el
@@ -842,6 +842,16 @@ ruby-ts--parent-call-or-bol
      ;; No paren/curly/brace found on the same line.
      ((< (treesit-node-start found) parent-bol)
       parent-bol)
+     ;; Nesting of brackets args.
+     ((and
+       (not (eq ruby-bracketed-args-indent t))
+       (string-match-p "\\`array\\|hash\\'" (treesit-node-type parent))
+       (equal (treesit-node-parent parent) found)
+       ;; Grandparent is not a parenless call.
+       (or (not (equal (treesit-node-type found) "argument_list"))
+           (equal (treesit-node-type (treesit-node-child found 0))
+                  "(")))
+      parent-bol)
      ;; Hash or array opener on the same line.
      ((string-match-p "\\`array\\|hash\\'" (treesit-node-type found))
       (save-excursion
diff --git a/test/lisp/progmodes/ruby-ts-mode-tests.el b/test/lisp/progmodes/ruby-ts-mode-tests.el
index 61ef80eb610..05d98974acf 100644
--- a/test/lisp/progmodes/ruby-ts-mode-tests.el
+++ b/test/lisp/progmodes/ruby-ts-mode-tests.el
@@ -326,6 +326,7 @@ "ruby-block-indent.rb"
 (ruby-ts-deftest-indent "ruby-method-call-indent.rb")
 (ruby-ts-deftest-indent "ruby-method-params-indent.rb")
 (ruby-ts-deftest-indent "ruby-parenless-call-arguments-indent.rb")
+(ruby-ts-deftest-indent "ruby-bracketed-args-indent.rb")
 
 (provide 'ruby-ts-mode-tests)
 

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

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Sun, Sep 1, 2024 at 8:19 PM Dmitry Gutov <dgutov@yandex.ru> 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

[-- Attachment #2: 0001-Add-ruby-bracketed-argument-indentation-option.patch --]
[-- Type: application/octet-stream, Size: 3217 bytes --]

From 8756d85448c0aee34279c18ae3430909a33645e4 Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 31 Aug 2024 19:31:20 -0400
Subject: [PATCH] Add ruby bracketed argument indentation option

* lisp/progmodes/ruby-mode.el (ruby-bracketed-args-indent),
(ruby-smie-rules): New option
* test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb:
* test/lisp/progmodes/ruby-mode-tests.el
  ("ruby-parenless-call-arguments-indent.rb"): New test case
---
 lisp/progmodes/ruby-mode.el                   | 24 ++++++++++++++
 .../ruby-bracketed-args-indent.rb             | 33 +++++++++++++++++++
 test/lisp/progmodes/ruby-mode-tests.el        |  1 +
 3 files changed, 58 insertions(+)
 create mode 100644 test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 3bcfa9ee7df..53d63c904a9 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -472,6 +472,26 @@ ruby-parenless-call-arguments-indent
   :safe 'booleanp
   :version "29.1")
 
+(defcustom ruby-bracketed-args-indent t
+  "Non-nil to align the contents of bracketed arguments with the brackets.
+
+Example:
+
+  qux({
+       foo => bar
+     })
+
+Set it to nil to align to the line with the opening bracket:
+
+  qux({
+    foo => bar
+  })
+
+Only has effect when `ruby-use-smie' is t."
+  :type 'boolean
+  :safe 'booleanp
+  :version "31.1")
+
 (defcustom ruby-deep-arglist t
   "Deep indent lists in parenthesis when non-nil.
 Also ignores spaces after parenthesis when `space'.
@@ -826,6 +846,10 @@ ruby-smie-rules
       ))
     (`(:before . ,(or "(" "[" "{"))
      (cond
+      ((and (not (eq ruby-bracketed-args-indent t))
+            (smie-rule-prev-p "," "(" "[")
+            (smie-rule-hanging-p))
+       (cons 'column (current-indentation)))
       ((and (equal token "{")
             (not (smie-rule-prev-p "(" "{" "[" "," "=>" "=" "return" ";" "do"))
             (save-excursion
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
new file mode 100644
index 00000000000..2d72aed6392
--- /dev/null
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby-bracketed-args-indent.rb
@@ -0,0 +1,33 @@
+foo
+  .update({
+    key => value,
+    other_key:
+  }, {
+    key => value,
+    other_key:
+  })
+
+update([
+  1,
+  2
+], [
+  3,
+  4
+])
+
+update([{
+  key: "value"
+}, {
+  key: "value"
+}])
+
+update(arg1, {
+  foo: "bar"
+}, [
+  1,
+  2
+], arg2)
+
+# Local Variables:
+# ruby-bracketed-args-indent: nil
+# End:
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index 2b8506a7adc..c9cde791baa 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -992,6 +992,7 @@ "ruby-block-indent.rb"
 (ruby-deftest-indent "ruby-method-call-indent.rb")
 (ruby-deftest-indent "ruby-method-params-indent.rb")
 (ruby-deftest-indent "ruby-parenless-call-arguments-indent.rb")
+(ruby-deftest-indent "ruby-bracketed-args-indent.rb")
 
 (ert-deftest ruby--test-chained-indentation ()
   (with-temp-buffer
-- 
2.42.1

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

[Dmitry Gutov]

On 02/09/2024 03:49, Aaron Jensen wrote:

>> 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 seems to me that anything other than 0 spaces would look inconsistent 
with the first element (the hash), and its closing brace in particular.

Anyway, it's not urgent and like you said in the first message, people 
formatting code this way might have other unusual requirements as well.

>> * 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 in advance.

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

[Aaron Jensen]

> It seems to me that anything other than 0 spaces would look inconsistent
> with the first element (the hash), and its closing brace in particular.

Yeah, that's my sense as well, even if it looks awful, but you get what you get.

> >> * 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 in advance.

I installed it and gave it a run on a few things. I didn't observe any
issues with it.

Aaron

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

[Dmitry Gutov]

On 02/09/2024 04:56, Aaron Jensen wrote:
>> It seems to me that anything other than 0 spaces would look inconsistent
>> with the first element (the hash), and its closing brace in particular.
> Yeah, that's my sense as well, even if it looks awful, but you get what you get.
> 
>>>> * 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 in advance.
> I installed it and gave it a run on a few things. I didn't observe any
> issues with it.

Great!

I've pushed both patches to emacs-30 (6c15b7710d4 and 24f12bdd77e) and 
now marking the issue as done. Thanks again for the patch.

To summarize for future readers: the default behavior doesn't change - 
at least not now - you need to customize the option for different 
indentation.

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

[Aaron Jensen]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

 Thank you, and thanks for your help.


Aaron

On Mon, Sep 2 2024 at 3:01 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 02/09/2024 04:56, Aaron Jensen wrote:
>
> It seems to me that anything other than 0 spaces would look inconsistent
> with the first element (the hash), and its closing brace in particular.
>
> Yeah, that's my sense as well, even if it looks awful, but you get what
> you get.
>
> * 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 in advance.
>
> I installed it and gave it a run on a few things. I didn't observe any
> issues with it.
>
> Great!
>
> I've pushed both patches to emacs-30 (6c15b7710d4 and 24f12bdd77e) and now
> marking the issue as done. Thanks again for the patch.
>
> To summarize for future readers: the default behavior doesn't change - at
> least not now - you need to customize the option for different indentation.
>

[-- Attachment #2: Type: text/html, Size: 1819 bytes --]