bug#33487: [PATCH] bug of ruby-forward-sexp and ruby-backward-sexp

bug#33487: [PATCH] bug of ruby-forward-sexp and ruby-backward-sexp

[Nobuyoshi Nakada]

Hello,


With lisp/progmodes/ruby-mode.el in Emacs 26.1, `ruby-forward-sexp' and 
`ruby-backward-sexp' cannot work with block arguments which end without 
argument name.

i.e.,

     proc do |a,|

     end


or


     proc do |a,*|

     end


The reason is that `smie-default-forward-token' and 
`smie-default-backward-token' just scan punctuation and the closing bar 
together, and return ",|" or ",*|" in the above examples, then the 
"opening-|" cannot find "matching closing-|".

The following patch trims the token and adjust the cursor position in 
those cases.


 From 318324f49f9bd0934bec691ed958a6776f190f09 Mon Sep 17 00:00:00 2001
From: Nobuyoshi Nakada <nobu@ruby-lang.org>
Date: Sun, 25 Nov 2018 00:49:57 +0900
Subject: [PATCH] Fix ruby-mode sexp at block arguments

Block arguments can end with ',' or '*' and a '|' may just follow
with no spaces.  At that arguments, `smie-default-forward-token'
and `smie-default-backward-token' just scan punctuations, and do
not extract the closing '|', which matches the opening '|'.
This also affected the indentation inside such blocks.
---
  lisp/progmodes/ruby-mode.el            |   4 +
  test/lisp/progmodes/ruby-mode-tests.el | 154 +++++++++++++++++++++++++
  2 files changed, 158 insertions(+)

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 9256dfc..c860393 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -517,6 +517,9 @@ ruby-smie--forward-token
               ((ruby-smie--opening-pipe-p) "opening-|")
               ((ruby-smie--closing-pipe-p) "closing-|")
               (t tok)))
+           ((string-match "\\`\\([^|]+\\)|+\\'" tok)
+            (forward-char (- (match-end 1)))
+            (match-string 1 tok))
             ((and (equal tok "") (looking-at "\\\\\n"))
              (goto-char (match-end 0)) (ruby-smie--forward-token))
             ((equal tok "do")
@@ -559,6 +562,7 @@ ruby-smie--backward-token
             ((ruby-smie--opening-pipe-p) "opening-|")
             ((ruby-smie--closing-pipe-p) "closing-|")
             (t tok)))
+         ((string-match-p "\\`\\([^|]+\\)|+\\'" tok) "closing-|")
           ((string-match-p "\\`|[*&]\\'" tok)
            (forward-char 1)
            (substring tok 1))
diff --git a/test/lisp/progmodes/ruby-mode-tests.el 
b/test/lisp/progmodes/ruby-mode-tests.el
index 72d83af..7520b17 100644
--- a/test/lisp/progmodes/ruby-mode-tests.el
+++ b/test/lisp/progmodes/ruby-mode-tests.el
@@ -718,6 +718,160 @@ ruby-sexp-test-example
      (ruby-backward-sexp)
      (should (= 2 (line-number-at-pos)))))

+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-no-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-no-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do$"))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-empty-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do ||
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-empty-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do ||
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-single-arg ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a|
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-single-arg ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a|
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-multiple-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,b|
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-multiple-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,b|
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-any-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |*|
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-any-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |*|
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-one-and-any-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,*|
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-one-and-any-args ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,*|
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
+(ert-deftest ruby-forward-sexp-skips-do-end-block-with-expanded-one-arg ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,|
+     |end")
+    (goto-char (point-min))
+    (forward-word 1)
+    (ruby-forward-sexp)
+    (should (= 2 (line-number-at-pos)))
+    (should (eolp))))
+
+(ert-deftest ruby-backward-sexp-skips-do-end-block-with-expanded-one-arg ()
+  (ruby-with-temp-buffer
+    (ruby-test-string
+     "proc do |a,|
+     |end")
+    (goto-char (point-min))
+    (end-of-line 2)
+    (ruby-backward-sexp)
+    (should (= 1 (line-number-at-pos)))
+    (should (looking-at "do "))))
+
  (ert-deftest ruby-toggle-string-quotes-quotes-correctly ()
    (let ((pairs
           '(("puts '\"foo\"\\''" . "puts \"\\\"foo\\\"'\"")
-- 
2.19.1

bug#33487: [PATCH] bug of ruby-forward-sexp and ruby-backward-sexp

[Dmitry Gutov]

Version: 27.0

Hi Nobuyoshi,

On 24.11.2018 18:20, Nobuyoshi Nakada wrote:
> With lisp/progmodes/ruby-mode.el in Emacs 26.1, `ruby-forward-sexp' and 
> `ruby-backward-sexp' cannot work with block arguments which end without 
> argument name.
> 
> i.e.,
> 
>      proc do |a,|
> 
>      end
> 
> 
> or
> 
> 
>      proc do |a,*|
> 
>      end
> 
> 
> The reason is that `smie-default-forward-token' and 
> `smie-default-backward-token' just scan punctuation and the closing bar 
> together, and return ",|" or ",*|" in the above examples, then the 
> "opening-|" cannot find "matching closing-|".
> 
> The following patch trims the token and adjust the cursor position in 
> those cases.

Thank you for the patch. I've simplified it a little: regexps smaller, 
tests shorter, and removed a few of the latter (seemed redundant), and 
pushed.

Please let me know if there is anything amiss in the resulting patch: 
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3729a3f88fd7ce1d8a1a7f2ea61e8c4d05e954ab