Re: Adding fix suggestions to Flymake diagnostics

[Eshel Yaron <me@eshelyaron.com>]

João Távora <joaotavora@gmail.com> writes:

> On Mon, May 27, 2024 at 1:15 PM Eshel Yaron <me@eshelyaron.com> wrote:
>
>> for example, it could rely on some overlay property that backends
>> would add to diagnostics and the UI of the new library would look
>> for.  If we really don't want to teach Flymake about fixes, I think
>> that could work.  WDYT?
>
> What you're describing is similar, if not exactly identical to what
> happens today

I'm describing/requesting a backend-agnostic UI for fixing diagnostics.
What happens in Eglot today is similar in that it's one example of a UI
for fixing diagnostics, but it's definitely a not generic one
(i.e. usable with other backends).

> 1. The overlay property you suggest exists.  It is 'keymap'
> 2. it's added to flymake diagnostics overlays via the diagnostic type property
>    'flymake-overlay-control', part of Flymake's generic API for adding things
>    to the overlay diagnostic.
> 3. The overlay _value_ is a simple keymap mapping the '[mouse-2]' binding to
>    a function.  I think some users add more mappings to the keymap, which is
>    bound to eglot-diagnostics-map.
> 4. The function is Eglot's.  It mixes UI and LSP logic, alas.
> [...]
> If you're suggesting to wrap all this setup in some function/macro of
> newlibrary.el (the file name for your "new library") so that only the
> complexity of 4.2 remains in eglot.el, then that's exactly what I'm
> suggesting.

More or less, yes.  Except the overlay property wouldn't be keymap but
something that just holds a function that produces the fix suggestion,
so as not to commit to a specific UI, like the fix-function argument of
flymake-make-diagnostic from my patch.

And again, I'm not especially interested in simplifying Eglot: that can
be a nice bonus, but it's fine IMO if Eglot ends up keeping its code
while also working with the standard API.

> I just note that newlibrary.el _will_ have to know about
> "edit-producing diagnostics", which means knowing about edits, or at
> least a way to apply them.  It will likely have to require
> 'refactor.el', which is where the "edit" format will live, and call on
> it to present and eventually apply those edits.
>
> In fact, in my mind, newlibrary.el is so short that there's little
> reason it shouldn't just be a small section of refactor.el.

Right.  We also needs to know about Flymake diagnostics and their
overlays, though, so the same argument suggests putting it in Flymake,
as I did in my draft implementation :)

But I think that we agree on the essence, and where the code lives isn't
that crucial anyway, so I don't mind moving this stuff to refactor.el or
indeed to a new library if that's preferable.