Re: Adding refactoring capabilities to Emacs

[João Távora]

>  Let's start working on this!

I'm posting a top-level reply to Eli's email with the content
of a reply to Dmitry in the scope of bug#60338.

That which concerns a significant enhancement to Eglot's
refactoring capability first proposed by Philip and finished by
myself.  It's closed.  So it's better to consolidate the
discussion here.

Especially I think that, to answer Eli's request successfully, we
have to take a look at the forest, not just one particular tree.

By the way, Stefan I CC'ed you so you could comment on the feasibility
of using (elisp--local-variables) for refactoring purposes, in
this case renaming.  Do we have source-tracking info there?

On Thu, Sep 7, 2023 at 2:00 AM Dmitry Gutov <dmitry@gutov.dev> wrote:
>
> On 02/09/2023 12:55, João Távora wrote:
> > Anyway I invite everyone to have a look and try to improve it, perhaps
> > moving it out of Eglot into the shiny new "refactoring interface" if
> > those ever become a thing.
>
> Regarding the diff approach vs. "shiny new", I've run a little
> comparison with VS Code: doing a rename of 'glyph_row' across the Emacs
> codebase (a moderately sized project) takes about ~1.44s to produce the
> diff.

FWIW, I got 0.85s on that same test on a GNU/Linux VM running on a 6
yo laptop with 4 cores assigned.  IMO this is more than acceptable for
this contrived example which renames a symbol across many files.
A much more typical example of renaming a symbol in a single function
resolved in 0.02s.  Most examples of applying quick fixes and moving
.hpp inlined member functions to out-of-line in .cpp resolve in similar
quasi-instantaneous fashion.

Now, let me clarify what I meant by "shiny new refactoring interface".
It's not what you think I meant, I believe. What I meant is a framework
for:

1. collecting 'potential sets of changes' -- for clarity let's call
   "actions" after LSP nomenclature -- from a number of different
   backends.  Examples of actions: "rename thing at point", "organize
   imports", "write as while loops as cl-loop"

2. presenting actions names to the user -- either via the minibuffer
   or by annotating buffer position where these tweaks apply.

3. after the user chooses an action, develop it into a set of changes
   -- again, for clarity and consonance with LSP nomenclature, we
   may call these "edits".

4. presenting these edits to the user via a number of different
   user-confirmation strategies.

For very common "rename" or "organize imports" actions, steps 1
and 2 may be coalesced and a special command may be provided that
goes directly to step 3.  LSP supports these "shortcuts".

Regarding 4 (which seems to be the current focal point of the
discussion) Eglot has three main types of confirmation strategies
available.  See eglot-confirm-server-edits for more.

* 'diff', which just presents the edits
* 'summary', which summarizes the edits in the minibuffer and prompts
  to apply.
* nil, which means no confirmation at all

There is also 'maybe-diff' and 'maybe-summary' which behave like
nil if all the files in the change list are already visited buffers.

That said, it's the whole sequence 1 to 4, not just 4, that needs
to be generalized out of Eglot and into Emacs.  Note that some parts
of 1..4, like the annotation of actions in buffers or the support for
file creation and deletion, aren't yet in Eglot, so there's good work
to be done.  Some of the "annotation" logic already exists in Flymake,
and Flymake must be taken into the equation too, and early.

Also, it would be very good if we could have an early backend which is
*not* LSP.  An obvious candidate is Elisp.  I'd like to know what is
the minimal effort to write a lisp function based on the new
macroexpanding tricks that Stefan Monnier developed that says
precisely where a given symbol is used as a variable in a function.
As far as I understand,  these techniques are currently being used for
accurate completion, but they could maybe be used for accurate
refactorings.

> Not to belittle the new addition, though - it's a good step for Eglot.

As someone who actually uses Eglot daily and is an avid consumer
of refactoring functionality, my opinion is that the "diff"
confirmation strategy that Philip proposed and implemented is freaking
great, even if the implementation is a bit contorted (though mostly
isolated).

IMO diff is the lingua franca for communicating changes to source
code around the world, even in those pretty web interfaces that
you and many others like.  So it makes full sense to support it
and to support it well.

> - I would probably want to bind the originally proposed
> 'diff-apply-everything' to 'C-c C-c' (e.g. "commit the change to disk"),
> but that's already taken in diff-mode.

> - 'git diff' has a less-frequently used option called '--word-diff'
> which could reduce the length of the diff in the case I am testing (but
> I guess diff-mode would have to be updated to support that output). And
> another way in that direction would be to reduce the size of the diff
> context (e.g. to 0).

I'm not at all opposed to implementing other confirmation strategies
that look more like what VSCode or other editors show users.  Or
augmenting the existing 'diff' strategy with new shortcuts and stuff.
But I wouldn't overthink UI details at this point in the game either.

João