From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.devel Subject: Re: Adding refactoring capabilities to Emacs Date: Thu, 31 Aug 2023 00:49:53 +0300 Message-ID: <0bba73a6-f9a5-91c8-ef56-a58951706490@gutov.dev> References: <83fs4f36wi.fsf@gnu.org> <61621120-8f6d-5884-e7c8-33581b8e0ca4@gutov.dev> <87edjkuv0s.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34743"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Cc: Yuri Khan , Eli Zaretskii , emacs-devel , "Philip K." To: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Aug 30 23:50:46 2023 Return-path: Envelope-to: ged-emacs-devel@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 1qbT5J-0008lY-TO for ged-emacs-devel@m.gmane-mx.org; Wed, 30 Aug 2023 23:50:46 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qbT4f-0000Np-UL; Wed, 30 Aug 2023 17:50:05 -0400 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 1qbT4d-0000NI-8Z for emacs-devel@gnu.org; Wed, 30 Aug 2023 17:50:03 -0400 Original-Received: from wout2-smtp.messagingengine.com ([64.147.123.25]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qbT4a-0006Mn-98; Wed, 30 Aug 2023 17:50:03 -0400 Original-Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 20FD43200911; Wed, 30 Aug 2023 17:49:58 -0400 (EDT) Original-Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Wed, 30 Aug 2023 17:49:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1693432197; x=1693518597; bh=O08l26+nvYYZlogoEDXPcbH7J+RTYUeRxq3 dwxf/xZc=; b=rQ7+ciHc3A91VzglZAF88jv2lswr0hYKrLGWzvBqOXMgbcREfPk QxpXadKysqu7Zcyoo5pLA0EFdFltXGcZgORCgwVAkCBFoW37uhilFC14W+1FE3sS USqj8PYU+yGkKWigD/cV/wVF21yqtgj3vIL+s70Wo8U8IT+Dtv154cBN0WejHv+A J4QXtuamFu9OXN+IxywfPUvwxkL5GAdX8DnxDYBmfti+zXFaGeTiTrMBd16O+qcI FimlnCmuBD9UhOA8Tq5fRujJ8f3VI98tTP08zK1M4Ib1WP6ywVcAMcX6xQgg9P7v PFPRuXgEBVmqcB/PCtWqfPg783S52uE3UOA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1693432197; x=1693518597; bh=O08l26+nvYYZlogoEDXPcbH7J+RTYUeRxq3 dwxf/xZc=; b=ED4o5re5PoCBhsDMLgJnOq0bRm04TVKMprVvue/qDP/mwFCnLCH THGGA5Bzd8XB/12y0CdAQmHt4GyAhKAktCl3kxU2nT7J8QfIwo6a4ILLZe88bn/K D82EfMliAqKm7PCug2cmbmBBTUOeH9Tuvdt2a1hoLHsH1ZfquoM6B/fkUKsphb6F uJhUltnmdAo5UfRIhUZlX6FKy3HqOS1omrSBUV7oHEPGiGuwFf3fqbIaC1d/ap6J dK8vyFXw44GOE8s2ObcEoaUcvlsu1FBiup3S9ynjBu/jttf4UWwkwsYIaIbFJ+ym TJu7RYcqPYkAvxNZeKc64JYW1EZf5uKUFBQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrudefledgtdduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtfeejnecuhfhrohhmpeffmhhi thhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrth htvghrnhepledvjeeuhfffueekkeegkeeugefhfeejjeffveektddvgeetteevueevkedv teffnecuffhomhgrihhnpegsohgssgihhhgrugiirdgtohhmnecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepughmihhtrhihsehguhhtohhvrdgu vghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 30 Aug 2023 17:49:56 -0400 (EDT) Content-Language: en-US In-Reply-To: <87edjkuv0s.fsf@gmail.com> Received-SPF: pass client-ip=64.147.123.25; envelope-from=dmitry@gutov.dev; helo=wout2-smtp.messagingengine.com X-Spam_score_int: -39 X-Spam_score: -4.0 X-Spam_bar: ---- X-Spam_report: (-4.0 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-1.242, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:309562 Archived-At: On 30/08/2023 23:37, João Távora wrote: > Dmitry Gutov writes: > >> On 29/08/2023 13:53, João Távora wrote: >>> On Sun, Aug 20, 2023, 23:52 Dmitry Gutov >> > wrote: >>> On . >>> What we don't have is any advanced UI coming with >>> that. Traditional >>> IDEs >>> (and apparently even VS Code now: >>> https://bobbyhadz.com/images/blog/rename-variable-vscode/refactor-preview.webp ) >>> have been featuring the "preview changes" feature for years. One where >>> you could see which files will be affected, and even opt out from some >>> of the changes. >>> It seems like the LSP protocol provides enough information for >>> this to >>> work (the response to the "rename" action is a list of changes to be >>> performed on the client), so the UI can definitely be extended there. >>> Philip K. has proposed a patch to Eglot that implements this in >>> bug#60338.  It is not without problems, but was generally agreeable >>> to me. Would you have a look, Dmitry? We stalled while thinking >>> about the user confirmation model... >> >> Hmmm. Some screenshots would go a long way, > > Just imagine a diff like *vc-diff*. Sure. >> I haven't tried the patch yet, but from what I can tell from the >> description, it's a pretty power-user-ish approach to UI. > > IMO showing a diff to a user such a is not a power-user-ish thing. Or > rather, it might be, but I don't see any better way to show potentially > complex code changes to users. > >> Similar to what we ended up doing with checkin-patch in VC -- >> powerful, but no very obvious to a non-pro user in how it can be >> operated. It is surely a good addition to Eglot, but the refactoring >> interface I was thinking of would have been more graphical (very >> vaguely in the style of Xref), looking a little closer to the VS Code >> screenshot I posted. > > I don't know how VSCode does it, but refactorings -- at least the kind > provided by LSP servers -- aren't in general as trivial as simple > renamings. They are arbitrary sets of changes to code, like moving > whole functions from one file to another, adding new sections or new > files, removing sections or whole files, etc, etc. I previously posted a link to a screenshot from VS Code, here it is again: https://bobbyhadz.com/images/blog/rename-variable-vscode/refactor-preview.webp Indeed, refactorings could be very arbitrary changes, though most of the time they are classified in certain types where each changes has some semantic meaning where the IDE previews knows how to render it (be it a renaming of a file, or of a function, or extracting a variable, etc). It's also helpful to show two related changes that are intermixed in the diff (e.g. when renaming a class also renames the file it's defined in). It might be useful to experiment with different changes (simpler and more complex) in VS Code and see what kind of previews it managed to show for them, naturally based on the data format returned by LSP. > In fact, they are just like patches. > > And I don't know any better way today to display patches to users other > than diff. > >> Regarding compatibility layer, I'm not sure I see a request-response >> mechanism here. I'd expect to see something synchronous, e.g. a call > > Synchronous is doable (Eglot does it, and the user shouldn't be doing > changes to the code while the refactoring backend is computing the > changes). It's possible that the backend malfunctions, or e.g. auto-revert-mode applies some changes while the user is still considering the changeset. > What I meant by request-proposal is that there is this > sequence > > 1. a request by the user selecting one of multiple offered > refactorings. > > 2. a synchronous response proposing changes > > 3. The decision by the user to apply some or all of these proposed > changes. This decision might be taken in advance in step 1, if the > user trusts the refactoring engine enough. > > Note: Files are not saved automatically, because this is a different > concern. "Not saving files automatically" is a power-user approach too. We have that in query-replace and naturally in xref-query-replace-in-results too, but having a large refactoring across many files end up in a not-synced-to-disk condition is a complication. Not everyone knows about save-some-buffers and auto-revert-mode.