From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#60655: 30.0.50; tree-sitter: `treesit-transpose-sexps' is broken. Date: Mon, 09 Jan 2023 13:29:12 +0100 Message-ID: <87cz7nuc13.fsf@thornhill.no> References: <87r0w5jo0i.fsf@masteringemacs.org> <87sfgkgqlj.fsf@thornhill.no> <878ricjdee.fsf@masteringemacs.org> Reply-To: Theodor Thornhill Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28375"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 60655@debbugs.gnu.org To: Mickey Petersen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jan 09 13:33:56 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pErLf-00079q-Tj for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 09 Jan 2023 13:33:56 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pErHw-0000lK-Md; Mon, 09 Jan 2023 07:30:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pErHu-0000jv-Mc for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 07:30:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pErHu-0007M0-Ck for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 07:30:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pErHu-0005op-3R for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 07:30:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Theodor Thornhill Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 09 Jan 2023 12:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60655 X-GNU-PR-Package: emacs Original-Received: via spool by 60655-submit@debbugs.gnu.org id=B60655.167326735822269 (code B ref 60655); Mon, 09 Jan 2023 12:30:02 +0000 Original-Received: (at 60655) by debbugs.gnu.org; 9 Jan 2023 12:29:18 +0000 Original-Received: from localhost ([127.0.0.1]:35966 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pErHB-0005n7-NI for submit@debbugs.gnu.org; Mon, 09 Jan 2023 07:29:18 -0500 Original-Received: from out-4.mta0.migadu.com ([91.218.175.4]:34220) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pErH9-0005mv-MI for 60655@debbugs.gnu.org; Mon, 09 Jan 2023 07:29:16 -0500 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=thornhill.no; s=key1; t=1673267354; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=7hMMt8sFthqDgWH4qtbi0FFipQpYkix4QrfUSZdZz6E=; b=yRA6pkT+ioTuZSI0HGgih5SAr6Xw+WAh447Ix0EXwvOVGlk2taj7KZ8V5LftknwvSinXtm g9wLjKdPhyy60BkmKwyNaIL0buA5db8QkZzQQaTdWlRC10BvLUrW+eTPhicTJme6rrxMWA wG3klI6j7NEUj8riIYPfWRTYkSsiQHcMvPKU+y4qNMT04RncZjI3xCIARlR6UK0TD1ivxd tI/AEpVzOTSrjDzhsl4q6mEbDsLbsM9ClfSO+ti7oJos5c/Ff+NV9S0+VbGwV1wAoCuNMw H9hRIx6twNctBsdULYFsNK4ZyA5LC6W9hrnKaIpeGhaT+pXO5pmSu8z70PXNXw== In-Reply-To: <878ricjdee.fsf@masteringemacs.org> X-Migadu-Flow: FLOW_OUT X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:253012 Archived-At: Mickey Petersen writes: > Theodor Thornhill writes: > >> Mickey Petersen > 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