bug#62086: 29.0.60; ruby-ts-mode regressions

[Dmitry Gutov]

On 05/04/2023 09:24, Juri Linkov wrote:
> > > I wonder is it possible to fix more.
> > > Many parens/brackets are still not matched in e.g.
> > > test/lisp/progmodes/ruby-mode-resources/ruby.rb
> > > such as parens in def argument list:
> > >     def test1(arg)
> >
> > This one was a regression from the addition of strict bos/eos anchors, now
> > fixed.
>
> Maybe there are more types that now are not found, but probably easier
> to add them one by one after testing than to try finding all of them in
> https://github.com/tree-sitter/tree-sitter-ruby/blob/master/src/node-types.json
> or in
> https://github.com/tree-sitter/tree-sitter-ruby/blob/master/src/grammar.json

Yep. And we've hopefully more-or-less covered the existing grammar at 
this point.

> > > and in
> > >     method (a + b),
> >
> > When you say that this is broken, do you mean that these parens get jumped
> > over unexpectedly (with forward-sexp movement ending at the end of the
> > arguments list)?
>
> It seems natural to expect that when point is on an opening paren/bracket
> then 'C-M-f' should jump to its closing pair.  At least, this is more WYSIWYG.
>
> > This is an artefact of the implementation of treesit-forward-sexp.
> > It might be possible to improve, but from a brief dig, it has some
> > internal logic. So some care would need to be taken to decide which
> > contract nedds changing.
>
> This is an example where explicit parens conflict with implicit parens.
> Visible parens have the type "parenthesized_statements", but invisible
> parens have the type "argument_list".  Both start at the same position.
> So maybe treesit-forward-sexp should prefer the former over the latter?
> And in a similar case
>
>    method [],
>           arg2
>
> maybe "array" should take precedence over "argument_list".

There is no mechanism for precedence in the current implementation. We 
can try ignoring the implicit parens in the parenless method calls, 
though. Like this:

diff --git a/lisp/progmodes/ruby-ts-mode.el b/lisp/progmodes/ruby-ts-mode.el
index ddf2ee98c3b..cf8f1b0d315 100644
--- a/lisp/progmodes/ruby-ts-mode.el
+++ b/lisp/progmodes/ruby-ts-mode.el
@@ -1086,6 +1086,15 @@ ruby-ts--syntax-propertize
            (put-text-property pos (1+ pos) 'syntax-table
                               (string-to-syntax "!"))))))))

+(defun ruby-ts--sexp-p (node)
+  ;; Skip parenless calls (implicit parens are both non-obvious to the
+  ;; user, and might take over when we want to just over some physical
+  ;; parens/braces).
+  (or (not (equal (treesit-node-type node)
+                  "argument_list"))
+      (equal (treesit-node-type (treesit-node-child node 0))
+             "(")))
+
 (defvar-keymap ruby-ts-mode-map
   :doc "Keymap used in Ruby mode"
   :parent prog-mode-map
@@ -1114,6 +1123,7 @@ ruby-ts-mode
   (setq-local treesit-defun-type-regexp ruby-ts--method-regex)

   (setq-local treesit-sexp-type-regexp
+              (cons
               (rx bol
                   (or "class"
                       "module"
@@ -1147,7 +1157,8 @@ ruby-ts-mode
                       "instance_variable"
                       "global_variable"
                       )
-                  eol))
+                  eol)
+                  #'ruby-ts--sexp-p))

   ;; AFAIK, Ruby can not nest methods
   (setq-local treesit-defun-prefer-top-level nil)



> > > Also square brackets are not matched by 'C-M-f' in
> > >     h[:key]
> >
> > And this, surprisingly, seems impossible to handle just using
> > treesit-sexp-type-regexp. The brackets are present in the tree, but they
> > are not at the ends of any node. So that will require some custom Lisp,
> > I guess.
>
> This is the same problem that occurs in other places such as in "#{ddf}"
> where only '#' but not '{' matches '}'.  So adding "element_reference"
> will allow to jump only from the beginning of an identifier.

Right, except it's worse because the identifier is usually much longer 
than one character.