GNU bug report logs - #73404
30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes

Previous Next

Full log

Message #103 received at 73404 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: theo <at> thornhill.no, Mickey Petersen <mickey <at> masteringemacs.org>,
 monnier <at> iro.umontreal.ca, 73404 <at> debbugs.gnu.org
Subject: Re: bug#73404: 30.0.50; [forward/kill/etc]-sexp commands do not
 behave as expected in tree-sitter modes
Date: Wed, 18 Dec 2024 20:04:11 -0800

> On Dec 17, 2024, at 11:37 PM, Juri Linkov <juri <at> linkov.net> wrote:
> 
> While testing treesit-forward-sexp-list, I discovered that
> thing-navigation functions are not restricted to named nodes.
> 
> I wonder if there a reason to find anonymous nodes as things?

We should rather ask is there any reason to not find anonymous nodes as things? Even ruby-ts-mode defines a bunch of anonymous nodes as sexp, no? In any case, excluding anonymous nodes from things doesn’t sound right.

> 
> The problem was found with the node "unless" in Ruby:
> 
>  unless cond
>    a += 1
>  else
>    b -= 1
>  end
> 
> Here the named node 'unless' has exactly the same name
> as the anonymous node with the text "unless":
> 
>  (unless "unless" condition: (identifier)

I feel like Ruby’s grammar should call the named node something else, like unless_statement.

> 
> Finding anonymous nodes breaks forward-sexp when point is on "unless":
> 
>  un-!-less cond
>    a += 1
>  else
>    b -= 1
>  end
> 
> because (treesit-thing-at (point) 'sexp t) finds
> 
>  #<treesit-node "unless" in 156-162>
> 
> instead of
> 
>  #<treesit-node unless in 156-203>
> 
> Also this breaks backward-sexp and backward-up-list
> because treesit--thing-sibling finds
> the anonymous node "unless" as a previous sibling
> instead of the named node 'unless' as a parent.
> 
> Would the right solution be to check if the found thing
> is a named node?  With something like:
> 
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index 18200acf53f..9ad879ee40c 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -2711,6 +2774,7 @@ treesit--thing-sibling
>                      (lambda (n) (>= (treesit-node-start n) pos))))
>          (iter-pred (lambda (node)
>                       (and (treesit-node-match-p node thing t)
> +                           (treesit-node-check node 'named)
>                            (funcall pos-pred node))))
>          (sibling nil))
>     (when cursor
> @@ -2760,6 +2824,7 @@ treesit-thing-at
>   (let* ((cursor (treesit-node-at pos))
>          (iter-pred (lambda (node)
>                       (and (treesit-node-match-p node thing t)
> +                           (treesit-node-check node 'named)
>                            (if strict
>                                (< (treesit-node-start node) pos)
>                              (<= (treesit-node-start node) pos))

A better solution IMO is to add some way to distinguish between named and anonymous nodes. I can think of two ways, either add “and” and “named/anonymous” predicate, so (and named “unless”) only matches the named “unless” node; or we add a special syntax such that “(unless)” only matches named nodes, and “\”unless\”” only matches anonymous nodes.

Yuan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.