bug#60655: 30.0.50; tree-sitter: `treesit-transpose-sexps' is broken.

[Theodor Thornhill]

Mickey Petersen <mickey@masteringemacs.org> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> Mickey Petersen <mickey@masteringemacs.org> > The tree-sitter-enabled
>> function, `treesit-transpose-sexps', that is called by
>> transpose-sexps, is broken.
>>>
>>> It uses a naive method of sibling adjacency to determine
>>> transpositions. But it is unfortunately not correct.
>>>
>>> Python:
>>>
>>>
>>>   def -!-foo():
>>>       pass
>>>
>>> Turns into this with `C-M-t':
>>>
>>>   def ()foo:
>>>       pass
>>>
>>> But it ought to be:
>>>
>>>   foo def():
>>>       pass
>>>
>>>
>>> It's swapping two siblings that are indeed adjacent in the tree, but
>>> not on screen, which is confusing and a regression from its previous
>>> behaviour.
>>>
>>
>> I can try to make transpose-sexps rely on only swapping "allowed"
>> node-types?  That would be able to keep the new, better function, yet
>> still disallow these syntax-breaking transposes.  What do you think?
>>
>
> This is a hard problem. I'm building the self-same in Combobulate, so
> when I saw this implementation I saw a well-trodden path by myself.
> There's a lot of subtlety to it, and it is not immediately possible to
> accurately gauge the right things to swap with simple (or not so
> simple) sibling transpositions.
>
> Using a defined list is better, but with the caveat that it requires manual
> intervention per mode. This is a really tricky thing to build well.

Yeah, but I guess that is a sensible change.  It isn't easy, no, so I'm
open for suggestions and improvements.  IMO an improvement would be to
increase the likelihood that a transpose-sexps will still be valid code.
I don't really think it is useful to do things like "def foo() -> foo
def()" because that is nonsensical code, and is covered by
transpose-words anyway.  To me a _more_ sensible approach here would be
to transpose the defun at point with the next one, as they are usually
interchangeable.  I am looking into such an improvement, and have been
for a while.

>
>
>
>>> You could make a cogent argument that both approaches are wrong from a
>>> syntactic perspective, but I think that misses the broader point that
>>> `C-M-t' now does something errant and unexpected.
>>
>> I don't really see how "foo def():" is any better at all.  We gain some
>> great improvements with this "naive" method - namely:
>>
>> if 5 + 5 == 10 then 10 else 100 + 100.  If point is on the else the 100
>> + 100 wil be swapped by 10, but the old behavior will be broken.
>>
>
> The old behaviour was consistent. It had a simple *modus operandi*:
> swap two things around point. As someone who has used `C-M-t' for
> decades, I know what it'll do in pretty much all situations, because I
> know what `C-M-k` and `C-M-f/b` do at all times.

It may be consistent, but imo it is too close to transpose-words, and
too likely to create useless code in non-lisp languages.

>
> Neither approach is great if you holistically approach this task as
> "making it correct at all times", and it is easy to confect scenarios
> that result in something that is semantically wrong, but syntactically
> correct; something that is plain wrong, both semantically and
> syntactically; and something that is occasionally correct.
>

I see what you mean, but to me semantically _and_ syntactically correct
is the benefit we should pursue when we actually have the parse tree.
The current implementation will semantically correct in many interesting
cases, such as the one I outlined, and is a huge improvement to the
current "transpose-words"-like behavior.  

> 'Like' siblings are an easy way out of this mess with the caveat, as
> you'll see, but now you need to carefully pluck the right nodes from
> the tree!
>
> Consider the node type `pair' in a dict in Python. They are easily 
> transposable for
> that very reason, notwithstanding the anonymous "," node betwixt them.
>
> That is why Combobulate has a list of stuff that it can safely
> transpose, and for everything else it defaults to the "classic"
> transpose.
>

Yeah, such an approach seems reasonable, and there is already precedence
in defining such "things" in Emacs.  As for the default fallback, I'll
see what I can do in the "treesit-transpose-sexps" function.  The
machinery in transpose-subr and friends is a little finicky, so to
adhere to that mechanism isn't the easiest thing.

>>>
>>> Worse, it's not possible to revert to the old behaviour (see
>>> bug#60654)
>>>
>>>
>
> Thanks for fixing that!
>

No problem - hopefully it is installed pretty soon.

Theo