unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#54702: 29.0.50; ruby-mode indentation: endless methods
@ 2022-04-04  2:03 Aaron Jensen
  2022-04-27  2:44 ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Jensen @ 2022-04-04  2:03 UTC (permalink / raw)
  To: 54702


Ruby 3 and 3.1 bring us different versions of endless methods. They
currently don't indent correctly in `ruby-mode`:

Current:

```rb
class Bar
  def foo = bar

    def baz
    end
  end
```

Expected:

```rb
class Bar
  def foo = bar

  def baz
  end
end
```
In GNU Emacs 29.0.50 (build 1, aarch64-apple-darwin21.3.0, NS appkit-2113.30 Version 12.2.1 (Build 21D62))
 of 2022-03-04 built on aaron-m1.local
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.3.1

Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/opt/homebrew/share/emacs/site-lisp
 --infodir=/opt/homebrew/Cellar/emacs-plus@29/29.0.50/share/info/emacs
 --prefix=/opt/homebrew/Cellar/emacs-plus@29/29.0.50 --with-xml2
 --with-gnutls --with-native-compilation --without-dbus
 --without-imagemagick --with-modules --with-rsvg --with-ns
 --disable-ns-self-contained 'CFLAGS=-I/opt/homebrew/opt/gcc/include
 -I/opt/homebrew/opt/libgccjit/include -I/opt/homebrew/opt/gmp/include
 -I/opt/homebrew/opt/jpeg/include' 'LDFLAGS=-L/opt/homebrew/lib/gcc/11
 -I/opt/homebrew/opt/gcc/include -I/opt/homebrew/opt/libgccjit/include
 -I/opt/homebrew/opt/gmp/include -I/opt/homebrew/opt/jpeg/include''

Configured features:
ACL GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix






^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-04-04  2:03 bug#54702: 29.0.50; ruby-mode indentation: endless methods Aaron Jensen
@ 2022-04-27  2:44 ` Dmitry Gutov
  2022-04-27 23:58   ` Aaron Jensen
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2022-04-27  2:44 UTC (permalink / raw)
  To: Aaron Jensen, 54702

Hi Aaron,

On 04.04.2022 05:03, Aaron Jensen wrote:
> 
> Ruby 3 and 3.1 bring us different versions of endless methods. They
> currently don't indent correctly in `ruby-mode`:
> 
> Current:
> 
> ```rb
> class Bar
>    def foo = bar
> 
>      def baz
>      end
>    end
> ```
> 
> Expected:
> 
> ```rb
> class Bar
>    def foo = bar
> 
>    def baz
>    end
> end
> ```

Thanks for the report.

I'll work on this further, but here's a quick-and-dirty patch to fix the 
indentation problems.

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index a197724634..3733603fb3 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -528,6 +528,9 @@ ruby-smie--forward-token
                (ruby-smie--forward-token)) ;Fully redundant.
               (t ";")))
             ((equal tok "&.") ".")
+           ((and (equal tok "def")
+                 (looking-at " *[a-zA-Z0-9_]* *\\(([^()]*)\\)? *="))
+            "def=")
             (t tok)))))))))

  (defun ruby-smie--backward-token ()
@@ -575,6 +578,9 @@ ruby-smie--backward-token
              (ruby-smie--backward-token)) ;Fully redundant.
             (t ";")))
           ((equal tok "&.") ".")
+         ((and (equal tok "def")
+               (looking-at "def *[a-zA-Z0-9_]* *\\(([^()]*)\\)? *="))
+          "def=")
           (t tok)))))))

  (defun ruby-smie--indent-to-stmt ()





^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-04-27  2:44 ` Dmitry Gutov
@ 2022-04-27 23:58   ` Aaron Jensen
  2022-12-16  0:33     ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Jensen @ 2022-04-27 23:58 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 54702

On Tue, Apr 26, 2022 at 10:44 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> Thanks for the report.
>
> I'll work on this further, but here's a quick-and-dirty patch to fix the
> indentation problems.

Great, thank you. I've since moved back to enh-ruby-mode and I was
able to patch it to support this (though that project appears to be
currently not accepting contributions). I'd probably use ruby-mode if
it supported indenting long parameter/argument lists the way the
non-smie version does, like this:

def some_method(
  some_param,
  some_other_param
)

I believe I've seen you imply concerns about enh-ruby-mode -- do you
have any aside from the fact that it's not in core and it requires a
ruby process? It's mostly worked well for me, but I don't know what I
don't know.

Thanks,

Aaron





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-04-27 23:58   ` Aaron Jensen
@ 2022-12-16  0:33     ` Dmitry Gutov
  2022-12-16  5:07       ` Aaron Jensen
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-16  0:33 UTC (permalink / raw)
  To: Aaron Jensen; +Cc: 54702

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

Hi Aaron,

Sorry for the long pause. You said you're using something else, though, 
so it didn't seem urgent (and I've yet to encounter endless methods at 
$day_job, FWIW).

On 28/04/2022 02:58, Aaron Jensen wrote:
> On Tue, Apr 26, 2022 at 10:44 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>> Thanks for the report.
>>
>> I'll work on this further, but here's a quick-and-dirty patch to fix the
>> indentation problems.
> 
> Great, thank you. I've since moved back to enh-ruby-mode and I was
> able to patch it to support this (though that project appears to be
> currently not accepting contributions).

Not a problem for me, but could you test the attached patch anyway?

It seems to handle a bunch of different/complex cases fine without 
regressions, but it's always better with a second pair of eyes.

> I'd probably use ruby-mode if
> it supported indenting long parameter/argument lists the way the
> non-smie version does, like this:
> 
> def some_method(
>    some_param,
>    some_other_param
> )

Now that the SMIE stuff is again in my short-term memory, it shouldn't 
be too hard. Just please file a separate bug report (slash feature 
request) with a precise example. Bonus points for linking to a relevant 
Rubocop rule, so that we can pick a better name for the new user option.

I don't see the non-SMIE version indenting it like this -- it looks more 
like this instead (and only if I set ruby-deep-indent-paren to nil):

def test2 (
     asd,
     asd
     asd
     )

So let's start with a couple of good examples.

> I believe I've seen you imply concerns about enh-ruby-mode -- do you
> have any aside from the fact that it's not in core and it requires a
> ruby process? It's mostly worked well for me, but I don't know what I
> don't know.

My main problem with it is the spotty maintenance like in this example: 
https://github.com/zenspider/enhanced-ruby-mode/issues/96

But it might work fine for many people. Especially those who don't use Robe.

Some previous versions of it (probably by the previous maintainer) were 
really broken, so I just took up ruby-mode instead. I haven't tried 
using its latest versions much.

[-- Attachment #2: ruby-endless-methods.diff --]
[-- Type: text/x-patch, Size: 4796 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 4ac289d529..4d4ca635a4 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -134,6 +134,12 @@ ruby-symbol-chars
 (defconst ruby-symbol-re (concat "[" ruby-symbol-chars "]")
   "Regexp to match symbols.")
 
+(defconst ruby-endless-method-head-re
+  (format " *%s+[?!]? *\\(([^()]*)\\)? *=" ruby-symbol-re)
+  "Regexp to match the beginning of an endless method definition.
+
+It should match the part after \"def\" and until \"=\".")
+
 (defvar ruby-use-smie t)
 (make-obsolete-variable 'ruby-use-smie nil "28.1")
 
@@ -351,7 +357,8 @@ ruby-smie-grammar
        (exp  (exp1) (exp "," exp) (exp "=" exp)
              (id " @ " exp))
        (exp1 (exp2) (exp2 "?" exp1 ":" exp1))
-       (exp2 (exp3) (exp3 "." exp3))
+       (exp2 (exp3) (exp3 "." exp3)
+             (exp3 "def=" exp3))
        (exp3 ("def" insts "end")
              ("begin" insts-rescue-insts "end")
              ("do" insts "end")
@@ -528,6 +535,9 @@ ruby-smie--forward-token
               (ruby-smie--forward-token)) ;Fully redundant.
              (t ";")))
            ((equal tok "&.") ".")
+           ((and (equal tok "def")
+                 (looking-at ruby-endless-method-head-re))
+            "def=")
            (t tok)))))))))
 
 (defun ruby-smie--backward-token ()
@@ -575,6 +585,9 @@ ruby-smie--backward-token
             (ruby-smie--backward-token)) ;Fully redundant.
            (t ";")))
          ((equal tok "&.") ".")
+         ((and (equal tok "def")
+               (looking-at (concat "def" ruby-endless-method-head-re)))
+          "def=")
          (t tok)))))))
 
 (defun ruby-smie--indent-to-stmt ()
@@ -641,9 +654,11 @@ ruby-smie-rules
        (unless (or (eolp) (forward-comment 1))
          (cons 'column (current-column)))))
     ('(:before . " @ ")
-     (save-excursion
-       (skip-chars-forward " \t")
-       (cons 'column (current-column))))
+     (if (smie-rule-parent-p "def=")
+         (smie-rule-parent)
+       (save-excursion
+         (skip-chars-forward " \t")
+         (cons 'column (current-column)))))
     ('(:before . "do") (ruby-smie--indent-to-stmt))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
@@ -672,6 +687,9 @@ ruby-smie-rules
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
           ruby-indent-level))
+    (`(:before . "=")
+     (and (smie-rule-parent-p " @ ")
+          (smie-rule-parent ruby-indent-level)))
     (`(:after . ,(or "?" ":")) ruby-indent-level)
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
@@ -1631,7 +1649,7 @@ ruby-add-log-current-method
                   (while (and (re-search-backward definition-re nil t)
                               (if (if (string-equal "def" (match-string 1))
                                       ;; We're inside a method.
-                                      (if (ruby-block-contains-point start)
+                                      (if (ruby-block-contains-point (1- start))
                                           t
                                         ;; Try to match a method only once.
                                         (setq definition-re module-re)
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index f39489071e..cbeb362ef0 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -500,3 +500,26 @@ def resolve(**args)
 
   member.call(**args)
 end
+
+# Endless methods.
+class Bar
+  def foo(abc) =
+    bar +
+    bar
+      .baz
+
+  def bar =
+    123 +
+    4
+
+  def request_params = {
+    headers: request_headers,
+    body: request_body
+  }
+end
+
+
+class Foo
+  def foo(...) = z
+  def bar = y
+end
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index e90a9e4075..9be01dc78f 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -605,6 +605,18 @@ ruby-add-log-current-method-after-inner-class-outside-methods-with-text
     (search-backward "FOO")
     (should (string= (ruby-add-log-current-method) "M::C"))))
 
+(ert-deftest ruby-add-log-current-method-after-endless-method ()
+  (ruby-with-temp-buffer (ruby-test-string
+                          "module M
+                          |  class C
+                          |    def foo =
+                          |      4_
+                          |  end
+                          |end")
+    (search-backward "_")
+    (delete-char 1)
+    (should (string= (ruby-add-log-current-method) "M::C#foo"))))
+
 (defvar ruby-block-test-example
   (ruby-test-string
    "class C

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16  0:33     ` Dmitry Gutov
@ 2022-12-16  5:07       ` Aaron Jensen
  2022-12-16 12:31         ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Jensen @ 2022-12-16  5:07 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 54702

On Thu, Dec 15, 2022 at 7:33 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Hi Aaron,
>
> Sorry for the long pause. You said you're using something else, though,
> so it didn't seem urgent (and I've yet to encounter endless methods at
> $day_job, FWIW).

Not a problem at all. Our project is very data heavy and we define
lots of one line methods, so we have made good use of them.

> Not a problem for me, but could you test the attached patch anyway?
>
> It seems to handle a bunch of different/complex cases fine without
> regressions, but it's always better with a second pair of eyes.

Sure. I tried a few things and the only problem I can find is that it
does not handle endless module methods:

def self.some_method =
  "some-value"

(with or without the line break, it handles them as the unpatched
version handles instance endless methods.

> > I'd probably use ruby-mode if
> > it supported indenting long parameter/argument lists the way the
> > non-smie version does, like this:
> >
> > def some_method(
> >    some_param,
> >    some_other_param
> > )
>
> Now that the SMIE stuff is again in my short-term memory, it shouldn't
> be too hard. Just please file a separate bug report (slash feature
> request) with a precise example. Bonus points for linking to a relevant
> Rubocop rule, so that we can pick a better name for the new user option.

Sure thing, just sent one in: bug#60110

> I don't see the non-SMIE version indenting it like this -- it looks more
> like this instead (and only if I set ruby-deep-indent-paren to nil):
>
> def test2 (
>      asd,
>      asd
>      asd
>      )
>
> So let's start with a couple of good examples.

Yeah, this is what I see too. Not sure what I saw before. In any case,
I sent a current/desired.

> My main problem with it is the spotty maintenance like in this example:
> https://github.com/zenspider/enhanced-ruby-mode/issues/96
>
> But it might work fine for many people. Especially those who don't use Robe.
>
> Some previous versions of it (probably by the previous maintainer) were
> really broken, so I just took up ruby-mode instead. I haven't tried
> using its latest versions much.

Copy that. It works well for me. My endless method patch has since been merged.

Unrelated, but I'm excited about the prospect of a treesit mode for Ruby.

Thanks again,

Aaron





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16  5:07       ` Aaron Jensen
@ 2022-12-16 12:31         ` Dmitry Gutov
  2022-12-16 12:40           ` Dmitry Gutov
  2022-12-16 13:12           ` Aaron Jensen
  0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-16 12:31 UTC (permalink / raw)
  To: Aaron Jensen; +Cc: 54702

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

On 16/12/2022 07:07, Aaron Jensen wrote:

>> Not a problem for me, but could you test the attached patch anyway?
>>
>> It seems to handle a bunch of different/complex cases fine without
>> regressions, but it's always better with a second pair of eyes.
> 
> Sure. I tried a few things and the only problem I can find is that it
> does not handle endless module methods:
> 
> def self.some_method =
>    "some-value"
> 
> (with or without the line break, it handles them as the unpatched
> version handles instance endless methods.

Right, thanks. See the attached updated patch.

>>> I'd probably use ruby-mode if
>>> it supported indenting long parameter/argument lists the way the
>>> non-smie version does, like this:
>>>
>>> def some_method(
>>>     some_param,
>>>     some_other_param
>>> )
>>
>> Now that the SMIE stuff is again in my short-term memory, it shouldn't
>> be too hard. Just please file a separate bug report (slash feature
>> request) with a precise example. Bonus points for linking to a relevant
>> Rubocop rule, so that we can pick a better name for the new user option.
> 
> Sure thing, just sent one in: bug#60110
> 
>> I don't see the non-SMIE version indenting it like this -- it looks more
>> like this instead (and only if I set ruby-deep-indent-paren to nil):
>>
>> def test2 (
>>       asd,
>>       asd
>>       asd
>>       )
>>
>> So let's start with a couple of good examples.
> 
> Yeah, this is what I see too. Not sure what I saw before. In any case,
> I sent a current/desired.

Thanks! The implementation (on top of the patch here) looks trivial, the 
question is whether to add an option, or just change the behavior and 
treat it like a "fix". Let's continue there after closing this one.

> Unrelated, but I'm excited about the prospect of a treesit mode for Ruby.

Yeah, it looks promising, but let's see how it goes.

And there is a certain barrier to compiling the tree-sitter stuff, so 
we'll probably need to maintain the current/pure version in parallel for 
a while.

[-- Attachment #2: ruby-endless-methods.diff --]
[-- Type: text/x-patch, Size: 4813 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 4ac289d529..d20e5e9b51 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -134,6 +134,12 @@ ruby-symbol-chars
 (defconst ruby-symbol-re (concat "[" ruby-symbol-chars "]")
   "Regexp to match symbols.")
 
+(defconst ruby-endless-method-head-re
+  (format " *\\(self\\.\\)?%s+[?!]? *\\(([^()]*)\\)? *=" ruby-symbol-re)
+  "Regexp to match the beginning of an endless method definition.
+
+It should match the part after \"def\" and until \"=\".")
+
 (defvar ruby-use-smie t)
 (make-obsolete-variable 'ruby-use-smie nil "28.1")
 
@@ -351,7 +357,8 @@ ruby-smie-grammar
        (exp  (exp1) (exp "," exp) (exp "=" exp)
              (id " @ " exp))
        (exp1 (exp2) (exp2 "?" exp1 ":" exp1))
-       (exp2 (exp3) (exp3 "." exp3))
+       (exp2 (exp3) (exp3 "." exp3)
+             (exp3 "def=" exp3))
        (exp3 ("def" insts "end")
              ("begin" insts-rescue-insts "end")
              ("do" insts "end")
@@ -528,6 +535,9 @@ ruby-smie--forward-token
               (ruby-smie--forward-token)) ;Fully redundant.
              (t ";")))
            ((equal tok "&.") ".")
+           ((and (equal tok "def")
+                 (looking-at ruby-endless-method-head-re))
+            "def=")
            (t tok)))))))))
 
 (defun ruby-smie--backward-token ()
@@ -575,6 +585,9 @@ ruby-smie--backward-token
             (ruby-smie--backward-token)) ;Fully redundant.
            (t ";")))
          ((equal tok "&.") ".")
+         ((and (equal tok "def")
+               (looking-at (concat "def" ruby-endless-method-head-re)))
+          "def=")
          (t tok)))))))
 
 (defun ruby-smie--indent-to-stmt ()
@@ -641,9 +654,11 @@ ruby-smie-rules
        (unless (or (eolp) (forward-comment 1))
          (cons 'column (current-column)))))
     ('(:before . " @ ")
-     (save-excursion
-       (skip-chars-forward " \t")
-       (cons 'column (current-column))))
+     (if (smie-rule-parent-p "def=")
+         (smie-rule-parent)
+       (save-excursion
+         (skip-chars-forward " \t")
+         (cons 'column (current-column)))))
     ('(:before . "do") (ruby-smie--indent-to-stmt))
     ('(:before . ".")
      (if (smie-rule-sibling-p)
@@ -672,6 +687,9 @@ ruby-smie-rules
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
           ruby-indent-level))
+    (`(:before . "=")
+     (and (smie-rule-parent-p " @ ")
+          (smie-rule-parent ruby-indent-level)))
     (`(:after . ,(or "?" ":")) ruby-indent-level)
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
@@ -1631,7 +1649,7 @@ ruby-add-log-current-method
                   (while (and (re-search-backward definition-re nil t)
                               (if (if (string-equal "def" (match-string 1))
                                       ;; We're inside a method.
-                                      (if (ruby-block-contains-point start)
+                                      (if (ruby-block-contains-point (1- start))
                                           t
                                         ;; Try to match a method only once.
                                         (setq definition-re module-re)
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index f39489071e..2e087393a8 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -500,3 +500,25 @@ def resolve(**args)
 
   member.call(**args)
 end
+
+# Endless methods.
+class Bar
+  def foo(abc) =
+    bar +
+    bar
+      .baz
+
+  def self.bar =
+    123 +
+    4
+
+  def request_params = {
+    headers: request_headers,
+    body: request_body
+  }
+end
+
+class Foo
+  def foo(...) = z
+  def bar = y
+end
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index e90a9e4075..9be01dc78f 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -605,6 +605,18 @@ ruby-add-log-current-method-after-inner-class-outside-methods-with-text
     (search-backward "FOO")
     (should (string= (ruby-add-log-current-method) "M::C"))))
 
+(ert-deftest ruby-add-log-current-method-after-endless-method ()
+  (ruby-with-temp-buffer (ruby-test-string
+                          "module M
+                          |  class C
+                          |    def foo =
+                          |      4_
+                          |  end
+                          |end")
+    (search-backward "_")
+    (delete-char 1)
+    (should (string= (ruby-add-log-current-method) "M::C#foo"))))
+
 (defvar ruby-block-test-example
   (ruby-test-string
    "class C

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 12:31         ` Dmitry Gutov
@ 2022-12-16 12:40           ` Dmitry Gutov
  2022-12-16 14:49             ` Eli Zaretskii
  2022-12-16 13:12           ` Aaron Jensen
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-16 12:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54702, Aaron Jensen

Hi Eli,

On 16/12/2022 14:31, Dmitry Gutov wrote:

> See the attached updated patch.

What do you think about installing this on emacs-29?

I want to treat this like a bugfix. It's fixing the lack of support for 
a language feature that came out 2 years ago, but is apparently growing 
in popularity now.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 12:31         ` Dmitry Gutov
  2022-12-16 12:40           ` Dmitry Gutov
@ 2022-12-16 13:12           ` Aaron Jensen
  2022-12-16 16:15             ` Dmitry Gutov
  1 sibling, 1 reply; 14+ messages in thread
From: Aaron Jensen @ 2022-12-16 13:12 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 54702

On Fri, Dec 16, 2022 at 7:31 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Right, thanks. See the attached updated patch.

That works. I found another inconsistency related to the other issue I
just opened:

def foo(
      baz,
      bar
) = what

def foo(
      baz,
      bar
    )
  hello
end

I don't know who is savage enough to do a multi-line endless method
like that, but when it's done the closing paren should probably be
consistent w/ the regular method closing paren.


> Yeah, it looks promising, but let's see how it goes.
>
> And there is a certain barrier to compiling the tree-sitter stuff, so
> we'll probably need to maintain the current/pure version in parallel for
> a while.

Indeed, it took me a while to even find reference to documentation on
it and I was looking explicitly, so there's work to be done there for
sure.

Aaron





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 12:40           ` Dmitry Gutov
@ 2022-12-16 14:49             ` Eli Zaretskii
  2022-12-18 12:06               ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2022-12-16 14:49 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 54702, aaronjensen

> Date: Fri, 16 Dec 2022 14:40:24 +0200
> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: 54702@debbugs.gnu.org, Aaron Jensen <aaronjensen@gmail.com>
> 
> Hi Eli,
> 
> On 16/12/2022 14:31, Dmitry Gutov wrote:
> 
> > See the attached updated patch.
> 
> What do you think about installing this on emacs-29?
> 
> I want to treat this like a bugfix. It's fixing the lack of support for 
> a language feature that came out 2 years ago, but is apparently growing 
> in popularity now.

If you think this is safe enough, it's fine with me.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 13:12           ` Aaron Jensen
@ 2022-12-16 16:15             ` Dmitry Gutov
  2022-12-16 16:24               ` Aaron Jensen
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-16 16:15 UTC (permalink / raw)
  To: Aaron Jensen; +Cc: 54702

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

On 16/12/2022 15:12, Aaron Jensen wrote:
> On Fri, Dec 16, 2022 at 7:31 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>>
>> Right, thanks. See the attached updated patch.
> 
> That works. I found another inconsistency related to the other issue I
> just opened:
> 
> def foo(
>        baz,
>        bar
> ) = what
> 
> def foo(
>        baz,
>        bar
>      )
>    hello
> end
> 
> I don't know who is savage enough to do a multi-line endless method
> like that, but when it's done the closing paren should probably be
> consistent w/ the regular method closing paren.

Thank you, savage indeed.

Okay, here's an alternative version -- this was a pain to implement.

Would be much easier if we just decided to change the args indentation 
without support for the current one.

[-- Attachment #2: ruby-endless-methods-v2.diff --]
[-- Type: text/x-patch, Size: 5108 bytes --]

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 4ac289d529..295a2a67e9 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -134,6 +134,12 @@ ruby-symbol-chars
 (defconst ruby-symbol-re (concat "[" ruby-symbol-chars "]")
   "Regexp to match symbols.")
 
+(defconst ruby-endless-method-head-re
+  (format " *\\(self\\.\\)?%s+[?!]? *\\(([^()]*)\\)? *=" ruby-symbol-re)
+  "Regexp to match the beginning of an endless method definition.
+
+It should match the part after \"def\" and until \"=\".")
+
 (defvar ruby-use-smie t)
 (make-obsolete-variable 'ruby-use-smie nil "28.1")
 
@@ -351,7 +357,8 @@ ruby-smie-grammar
        (exp  (exp1) (exp "," exp) (exp "=" exp)
              (id " @ " exp))
        (exp1 (exp2) (exp2 "?" exp1 ":" exp1))
-       (exp2 (exp3) (exp3 "." exp3))
+       (exp2 (exp3) (exp3 "." exp3)
+             (exp3 "def=" exp3))
        (exp3 ("def" insts "end")
              ("begin" insts-rescue-insts "end")
              ("do" insts "end")
@@ -528,6 +535,9 @@ ruby-smie--forward-token
               (ruby-smie--forward-token)) ;Fully redundant.
              (t ";")))
            ((equal tok "&.") ".")
+           ((and (equal tok "def")
+                 (looking-at ruby-endless-method-head-re))
+            "def=")
            (t tok)))))))))
 
 (defun ruby-smie--backward-token ()
@@ -575,6 +585,9 @@ ruby-smie--backward-token
             (ruby-smie--backward-token)) ;Fully redundant.
            (t ";")))
          ((equal tok "&.") ".")
+         ((and (equal tok "def")
+               (looking-at (concat "def" ruby-endless-method-head-re)))
+          "def=")
          (t tok)))))))
 
 (defun ruby-smie--indent-to-stmt ()
@@ -629,6 +642,13 @@ ruby-smie-rules
                      (not (ruby-smie--bosp)))
            (forward-char -1))
          (smie-indent-virtual))
+        ((and (smie-rule-parent-p " @ ")
+              (save-excursion
+                (goto-char (nth 1 (smie-indent--parent)))
+                (and
+                 (smie-rule-prev-p "def=")
+                 (smie-indent-backward-token)
+                 (smie-indent-virtual)))))
         (t (smie-rule-parent))))))
     (`(:after . ,(or "(" "[" "{"))
      ;; FIXME: Shouldn't this be the default behavior of
@@ -672,6 +692,11 @@ ruby-smie-rules
      (and (smie-rule-parent-p ";" nil)
           (smie-indent--hanging-p)
           ruby-indent-level))
+    (`(:before . "=")
+     (when (smie-rule-parent-p " @ ")
+       (goto-char (nth 1 (smie-indent--parent)))
+       (smie-indent-backward-token)
+       (cons 'column (+ (smie-indent-virtual) ruby-indent-level))))
     (`(:after . ,(or "?" ":")) ruby-indent-level)
     (`(:before . ,(guard (memq (intern-soft token) ruby-alignable-keywords)))
      (when (not (ruby--at-indentation-p))
@@ -1631,7 +1656,7 @@ ruby-add-log-current-method
                   (while (and (re-search-backward definition-re nil t)
                               (if (if (string-equal "def" (match-string 1))
                                       ;; We're inside a method.
-                                      (if (ruby-block-contains-point start)
+                                      (if (ruby-block-contains-point (1- start))
                                           t
                                         ;; Try to match a method only once.
                                         (setq definition-re module-re)
diff --git a/test/lisp/progmodes/ruby-mode-resources/ruby.rb b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
index f39489071e..9d9b46fa31 100644
--- a/test/lisp/progmodes/ruby-mode-resources/ruby.rb
+++ b/test/lisp/progmodes/ruby-mode-resources/ruby.rb
@@ -500,3 +500,41 @@ def resolve(**args)
 
   member.call(**args)
 end
+
+# Endless methods.
+class Bar
+  def foo(abc) =
+    bar +
+    bar
+      .baz
+
+  def self.bar =
+    123 +
+    4
+
+  def request_params = {
+    headers: request_headers,
+    body: request_body
+  }
+
+  foo a = 5,
+      b
+
+  def foo(
+        baz,
+        bar
+      ) =
+    what
+
+  def foo(
+        baz,
+        bar
+      )
+    hello
+  end
+end
+
+class Foo
+  def foo(...) = z
+  def bar = y
+end
diff --git a/test/lisp/progmodes/ruby-mode-tests.el b/test/lisp/progmodes/ruby-mode-tests.el
index e90a9e4075..9be01dc78f 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -605,6 +605,18 @@ ruby-add-log-current-method-after-inner-class-outside-methods-with-text
     (search-backward "FOO")
     (should (string= (ruby-add-log-current-method) "M::C"))))
 
+(ert-deftest ruby-add-log-current-method-after-endless-method ()
+  (ruby-with-temp-buffer (ruby-test-string
+                          "module M
+                          |  class C
+                          |    def foo =
+                          |      4_
+                          |  end
+                          |end")
+    (search-backward "_")
+    (delete-char 1)
+    (should (string= (ruby-add-log-current-method) "M::C#foo"))))
+
 (defvar ruby-block-test-example
   (ruby-test-string
    "class C

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 16:15             ` Dmitry Gutov
@ 2022-12-16 16:24               ` Aaron Jensen
  2022-12-16 17:49                 ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Jensen @ 2022-12-16 16:24 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 54702

On Fri, Dec 16, 2022 at 11:15 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 16/12/2022 15:12, Aaron Jensen wrote:
> > On Fri, Dec 16, 2022 at 7:31 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
> >>
> >> Right, thanks. See the attached updated patch.
> >
> > That works. I found another inconsistency related to the other issue I
> > just opened:
> >
> > def foo(
> >        baz,
> >        bar
> > ) = what
> >
> > def foo(
> >        baz,
> >        bar
> >      )
> >    hello
> > end
> >
> > I don't know who is savage enough to do a multi-line endless method
> > like that, but when it's done the closing paren should probably be
> > consistent w/ the regular method closing paren.
>
> Thank you, savage indeed.
>
> Okay, here's an alternative version -- this was a pain to implement.
>
> Would be much easier if we just decided to change the args indentation
> without support for the current one.

It works for me w/ that example.

You won't find me resisting getting rid of the old way. Lining up
against the method name is a fairly clear UX issue, in my opinion. I
don't know that it's something I see outside of Emacs, and my guess is
that it was influenced by Lisp rather than being influenced by Ruby
and its community.

Aaron





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 16:24               ` Aaron Jensen
@ 2022-12-16 17:49                 ` Dmitry Gutov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-16 17:49 UTC (permalink / raw)
  To: Aaron Jensen; +Cc: 54702

On 16/12/2022 18:24, Aaron Jensen wrote:

>> Okay, here's an alternative version -- this was a pain to implement.
>>
>> Would be much easier if we just decided to change the args indentation
>> without support for the current one.
> 
> It works for me w/ that example.
> 
> You won't find me resisting getting rid of the old way. Lining up
> against the method name is a fairly clear UX issue, in my opinion. I
> don't know that it's something I see outside of Emacs,

I suppose we might still want to care about a bunch of Emacs users who 
got used to this indentation over the years. :/

> and my guess is
> that it was influenced by Lisp rather than being influenced by Ruby
> and its community.

You could say that.

Not really influenced, though, it just works that way by accident due to 
the structural indentation algorithm.

There are other Lispy examples which seem to provide their value:

   foo = 3 + 4 *
             5

or this example, which is influenced by a lot of early Ruby code 
examples, yet is not supported by a lot of editors these days:

   qux :+,
       bar,
       :[]=,
       bar,
       :a






^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-16 14:49             ` Eli Zaretskii
@ 2022-12-18 12:06               ` Dmitry Gutov
  2022-12-18 15:42                 ` Aaron Jensen
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2022-12-18 12:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 54702-done, aaronjensen

Version: 29.1

On 16/12/2022 16:49, Eli Zaretskii wrote:
>> Date: Fri, 16 Dec 2022 14:40:24 +0200
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Cc: 54702@debbugs.gnu.org, Aaron Jensen <aaronjensen@gmail.com>
>>
>> Hi Eli,
>>
>> On 16/12/2022 14:31, Dmitry Gutov wrote:
>>
>>> See the attached updated patch.
>>
>> What do you think about installing this on emacs-29?
>>
>> I want to treat this like a bugfix. It's fixing the lack of support for
>> a language feature that came out 2 years ago, but is apparently growing
>> in popularity now.
> 
> If you think this is safe enough, it's fine with me.

Thank you.

After some testing, I went with the v2 patch, further simplified.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#54702: 29.0.50; ruby-mode indentation: endless methods
  2022-12-18 12:06               ` Dmitry Gutov
@ 2022-12-18 15:42                 ` Aaron Jensen
  0 siblings, 0 replies; 14+ messages in thread
From: Aaron Jensen @ 2022-12-18 15:42 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Eli Zaretskii, 54702-done

On Sun, Dec 18, 2022 at 7:06 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
> Thank you.
>
> After some testing, I went with the v2 patch, further simplified.

Thank you.





^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-12-18 15:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04  2:03 bug#54702: 29.0.50; ruby-mode indentation: endless methods Aaron Jensen
2022-04-27  2:44 ` Dmitry Gutov
2022-04-27 23:58   ` Aaron Jensen
2022-12-16  0:33     ` Dmitry Gutov
2022-12-16  5:07       ` Aaron Jensen
2022-12-16 12:31         ` Dmitry Gutov
2022-12-16 12:40           ` Dmitry Gutov
2022-12-16 14:49             ` Eli Zaretskii
2022-12-18 12:06               ` Dmitry Gutov
2022-12-18 15:42                 ` Aaron Jensen
2022-12-16 13:12           ` Aaron Jensen
2022-12-16 16:15             ` Dmitry Gutov
2022-12-16 16:24               ` Aaron Jensen
2022-12-16 17:49                 ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).