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

[Dmitry Gutov <dgutov@yandex.ru>]

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.