Subject: Re: bug#50244: 28.0.50; Support project-wide diagnostics reports in flymake.el

From: João Távora <[email-masked]>
Date: Sat, 23 Oct 2021 21:22:20 +0100

On Sat, Oct 23, 2021 at 8:22 PM Theodor Thornhill <[email-masked]> wrote:
> (sent this mail some days ago, but the bug was archived, so not sure if
> it worked.  Trying again.  If this is only noise, then I'm sorry, but
> hopefully ok considering i found a typo in the old patch :))

No problem, and yes I got the other one.  It's just I've been again
ompletely utterly busy so it'll be a while.  I took the Flymake thing out
of my brain cache and it's going to take a while to get it back.

> I finally did get to test this out a little, and I seem to have hit some
> small snags I can't really understand.  I'd love some pointers as to how
> to continue!
> (the patch I currently use is attached at the bottom)
> Ok, so what happens is:
>
> - There is no backend set on the list-only-diagnostics as for now, and I
>   cannot for the life of me see how to set them, as it doesn't look like
>   eglot itself sets it.  Is there some reflection going on here that I
>   have missed?

No, I don't think so.  Maybe I simply decided that no backend was
necessary?  Is it?  If it is, we might to add some mechanism so
that it is assigned.

> - The function in question is riddled with fallbacks and failure
>   checking.  Are we sure we need all that?  I see most of the commits
>   relating to the range checking is from 2018, so they may not be
>   relevant anymore?

I'm not sure I follow here.  I wouldn't delete any checks until I'm
sure they are doing nothing. 2018 or not, I do remember a lot of
servers providing very strange diagnostics and invalid ranges.

> - The diff as supplied appends the list-only-diagnostics just fine to
>   the flymake buffer, but I believe since the backend isn't set, eglot
>   doesn't report the proper diagnostics when I visit the file.  I have
>   to make a bogus edit to trigger the eglot-flymake-backend function.
>   (This could just as well be some unrelated eglot/elmls issue...)

Ah, this rings a bell. You're talking about visiting a file by clicking a
"list-only  diagnostic" in the list, right?  And then you wnat Flymake to
resume checking and replace them with "real" ones.  Yes, might be
eglot/elmls issue, but usually there's not much we can do: some servers
are like that.

To keep things consistent we could remove the relevant list-only
diagnostics and re-report them via the normal mechanism if the server
is taking too long.

Do you understand this idea? The user wouldn't, in principle, be able to
tell that the freshly annotated diagnostics weren't collected just now
when he visited the buffer, but much much earlier.  The project-wide
listing should, in principle, also remain consistent.

...though probably unsorted.  If I remember correctly consistent sorting
is still a problem.

> - What would be the best way to get the proper line and column?  Now I
>   simply use the range, but considering there's a lot of error handling
>   there I guess that is a little risky?  I did have a version earlier
>   using with-temp-buffer and finding the correct positions, but that
>   seemed more complicated than needed.

Risky for what?  I don't follow

If you used with-temp-buffer, I guess you visited the file.  That would
go against the idea of list-only-diagnostics, which are supposed
to be for unvisited files (if the file were visited you'd have Eglot/Flymake
actually checking).  So using the LSP range and converting it to
flymake-make-diagnostic's protocol seems ok.

> I'd love to get your thoughts on this :)

Hope I could help and sorry if the thoughts were a little sparse.
As I said, this is out of my cache and will be for some time.

Do consider patches/changes to flymake.el as well. This new
mechanism is very new, it's possible it needs some tweaks.

João