bug#66552: 30.0.50; Eglot feature request: handle quirky code actions

[João Távora]

On Tue, Oct 17, 2023 at 8:08 PM Richard Copley <rcopley@gmail.com> wrote:

> > If a servers sends no field, Eglot will
> > ignore it.  If it sends a wrong one, Eglot won't.
>
> > You could work around it with a monkey-patch by advising
> > eglot--apply-text-edits to always have a null second argument.
>
> I'll probably go with that. Thanks for the suggestion. It's a shame
> that this will affect all languages. It would be nice if I could use a
> defmethod dispatching on a subclass of eglot-lsp-server! But
> eglot--apply-text-edits doesn't take a server, so it isn't generic.

For eglot--apply-text-edits to become a generic, it would first
have to be promoted to Eglot's user API (losing the --), and I
don't see a very good reason to do that right now.

However, if you wanted a fully "legal" solution, I think you could
place a server-specific method on

  eglot-handle-request (server your-server) (method workspace/applyEdit)

which removes or massages the `version` cookie in the `edit` keyword
argument.

And even with the monkey-patching approach, you can make something
server specific by checking the return value of `eglot-current-server`
before tweaking the value.

Another possibility is for Eglot to start interpreting version 0 as
"any version".  It should be feasible since Eglot controls the
start of the numbering, which right now is 0 but be increased to 1,
hopefully without any negative consequences.

> > The second labeling problem is even more bizarre  Again, you
> > can probably monkey-patch Eglot to work around it.
>
> Here I'm not sure I agree. The title is only specified to be "A short,
> human-readable, title for this code action". In this case the
> suggestions are computed, and inventing unique friendly names for them
> isn't feasible. "Apply 'Try this'" seems sensible. It refers to the
> text of the corresponding diagnostic, "Try this : <newText>".

This newText is hidden deep inside the `edits` field of such messages.
There's no guarantee it's a 1-element array or even that the edit
is just an insertion.  So grabbing it consistently to craft a
user-visible message is just a bad idea

It doesn't make sense for a language server to provide a bunch
of actions with identical labels.  It isn't something the client
should have to deal with.  A label, to me, is a piece of information
for telling one object apart from another object.  If the text of the
corresponding diagnostic is (presumably unique), why can't
the label be unique as well?

João