emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Adding fix suggestions to Flymake diagnostics


From: Eshel Yaron
Subject: Re: Adding fix suggestions to Flymake diagnostics
Date: Fri, 31 May 2024 09:15:29 +0200
User-agent: Gnus/5.13 (Gnus v5.13)

Hi,

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

> On Mon, May 27, 2024 at 6:40 PM Eshel Yaron <me@eshelyaron.com> wrote:
>
>> 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.
>
> That's a shame, but your call. Eglot will have no more code to do the
> same it already does.  I hope you've gotten my input of how I'd do it,
> you're free to do it as you wish and I'm free to ignore it.

I got your input and I appreciate your time and thought.  Thanks.

To avoid getting too caught up on the specifics of Eglot, we can use
another Flymake backend to demonstrate how a standard fix suggestions
feature might look.  I'm attaching below a new patch that adds fix
suggestions for shellcheck diagnostics in sh-mode via the same API.

Here's an example shell script you can use to see how it works:

--8<---------------cut here---------------start------------->8---
#!/bin/bash

echo foo/$1/baz
--8<---------------cut here---------------end--------------->8---

Shellcheck complains that $1 better be quoted, and we see that via a
Flymake diagnostic.  Go to that diagnostic and say M-x flymake-fix RET
to fix it.  (This still uses refactor.el for applying the code edits.)

>> > 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 :)
>
> It's not a symmetrical situation.  It's quite different to use the
> longstanding battle-tested, 4-major-version-old concept of "diagnostic"
> in a new library and taiting a longstanding stable library with a
> duplicated, brittle,  untested new concept that is completely foreign
> to its genesis.

Flymake communicates with programs that produce diagnostics, along with
fixes that describe suggested code edits.  I agree that code edits are
not the core business of Flymake, but characterizing code edits as
"completely foreign to Flymake's genesis" seems somewhat exaggerated.

Anyway, here's that new patch:

Attachment: shellcheck-fix.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]