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

["João Távora" <joaotavora@gmail.com>]

Dmitry Gutov <dgutov@yandex.ru> writes:

> It's a judgment call, but that seems fairly involved for a public API.

Certainly no more complicated than what the Flymake API already has,
what with synchronous and asynchronous invocation of callbacks with
specific arguments.  However if something justifies it, though, then
flymake-add/remove-list-only-diagnostics can be created to hide the
implemetation.

> That's why I was thinking of a somewhat different shape: where a
> backend reports a full list (corresponding to the default-directory
> it's called from), and then Flymake can either show the list, or
> discard it at some later point.

When do they report it.  In what moments?  Under what circumstances?
You address this questions, but only vaguely, at the end of your email.

> Then you won't need project.el to filter the full global list,
> delegating the job of compiling the list to the backend. That seems to
> be the most flexible scheme: some authors will prefer Projectile or
> whatever, but it some cases the backend will simply have a different
> view into what a project it (which the user's config might or might
> not reflect). Subtly losing diagnostics in such scenarios seems like a
> bad outcome.

I guess I can provide the full list of diagnostics for a user to plug in
whatever filtering function she wants.  But project filtering is what's
at stake in this request and the project library in Emacs is project.el.
I'd rather use project.el than anything else.  And rather use in the
framework, hidden from the backend's view, than talk to backends about
projects, which they know nothing about.  So it was a very simple
decision there.

> Maybe I could say that we could have two types of diagnostics (maybe
> even just one: with file-based locations), but they would be
> differentiated by the source them came from. Some would be for the
> current buffer, and some would be for all buffers.

This seems reasonably close to what the existing concepts of "foreign"
and "list-only" diagnostics is.

> IIUC LSP protocol provides some kind of feed where the client can
> subscribe to the updates, and then it sends diagnostics with some
> regularity, where latency probably varies by language server.

If I understand you correctly, that's not at all the way the the LSP
protocol handles diagnostics.  Diagnostics are sent by the server when
it feels like it.  But normally, almost 100%, they are sent by the
server very shortly after the client informs about a change in the
"virtual file".  There is no subscription.

> But in any case such syntax checks much be triggered by edits and
> saving of project files.
>
> It would be nice if 'M-x flymake-show-diagnostics-buffer' would
> provide basically the same view into that data: it would update with
> higher delays, but otherwise show the same data. Not sure if LSP can
> send reports like "syntax checking in progress" -- if so, perhaps the
> view could also reflect that.

But I don't understand what the problem with the current solution is.
Both f-s-buffer-diagnostics and f-s-project-diagnostics auto-update with
"edits and saves".

> I suppose some users would prefer to be able to show/hide info not
> pertaining to the current buffer, but that's a matter of basic
> filtering.

Ah! I guess the two things could be merged into the same command,
indeed.  That's a good idea.  Unfortunately there's the fact that when
you narrow to one buffer, the file column becomes useless.  Hiding a
column not easily accomplished in the current tabulated-list-mode
infrastructure.  But that's just a technical hurdle.

>> Currently, what _can_ be seen in the logs is an LSP server shooting out
>> a once-only batch of diagnostics for the whole project of unvisited
>> files when the server is started.  That is better suited to "list-only"
>> diagnostics.  Why "list only"?  Because once the actual file is visited,
>> it is assumed that flymake-mode will kick in there proper and be able to
>> request fresh, domestic, highlightable, diagnostics to override the
>> initial "list only" that came in the starting batch.
>
> I suppose the assumption is that the "list only" diagnostics cannot be
> highlightable?

Ye.... no.  That's their _definition_.  They are only for files which
are not yet visited, so there's nothing to highlight in the moment of
their creation.  The _assumption_ is rather that when their files are
eventually visited by Emacs and flymake-mode, we _assume_ that the
regular ensuing syntax checks deriving from the visit and or from edits
to those already brings in diagnostics.  And that is a good assumption,
if there's a traditional flymake-diagnostic-function capable of handling
that file, which there normally is.  This requires no change to that
existing part of the backend, which was desirable.

>>> No code, sorry.
>> If you want a new abnormal hook (as you seemed to propose), you have
>> to
>> say, at least in some kind of pseudo-code, _when_ that hook is going to
>> be activated.  Maybe by doing that I could start to see the problem that
>> it is solving (I also don't see that).
>
> There are options. If the list-only diagnostics will only be displayed
> in the show-diagnostics buffer (and not in the source code buffers),
> the hook could run inside the *Flymake diagnostics* buffer the first
> time it is initiated. Or flymake-global-diagnostic-functions (let's
> call it that) could be called right away when a file is visited.
>
> And maybe it would be called again and again often: whenever the
> current buffer changes, or every second after that. With the
> assumption that calling it too often would not be a problem, would not
> break/restart syntax checking processes (which is the case for LSP,
> for instance), it would just notify each global diagnostics backend
> whether default-directory has changed, and simply ask for updates
> (with the possibility of optimization that the backend would return
> lists which are 'eq' to each other if the list hasn't changed yet).
>
> Alternatively, we could go further than the approach of hooks and make
> the global sources of diagnostics more like subscriptions. In that
> case they would be passed callbacks which are supposed to be called
> many time: every time the diagnostics change. The extra questions are
> how to "unsubscribe" and "relocate" (change default-directory), and
> whether to always "relocate" by "unsubscribing" +
> "subscribing". Anyway, these functions can still reside in a hook,
> with the general shape of each functions like
>
>   (lambda (action &optional multi-callback))
>
> Where ACTION is one of `subscribe', `unsubscribe' or `relocate'. And
> DEFAULT-DIRECTORY serving as an implicit argument, though it can be
> made explicit just as well.
>
> I'm not saying this feature is easy or anything, 

I have no opinion.  It is certainly interesting.  Subscriptions,
relocations... It spikes my curiosity to read this, but no more than
that.  It tells me that you have ideas and have put in the effort to
write several paragraphs.  But I cannot see clearly the problem that you
are trying to solve.  So I cannot see value in your idea.  Nor can I
really criticize it, to be honest.  Maybe someone else will, of course,
and it's good that you wrote it down.

In contrast, I was able to see Theodor's problem clearly.  Why? Because
he made his own implementation that solves his problem so it's very very
easy to see what he means and to address what he means.  So even if we
have completely discarded that implementation, it was super-useful for
illustration.  That is not the only way to communicate software ideas,
of course, but it is certainly one of the most effective.  It's one
where many "thinkos" that are only natural when imagining vapourware
solutions are already sorted, because reality sorted them for you.  It
happens a lot with me: imagining this and having to discard them once I
"hit the metal".

> but it would be nice to avoid having multiple sources editing the same
> alist at once (that logic should be part of the framework IMHO, based
> on the data the sources provide), and to let the sources themselves
> decide which files are part of their "project view". They can still
> use project.el underneath, if they find that convenient.

Two points:

* I don't see why multiple sources editing a map is a bad thing in
  itself.  Why is it a bad thing?  Just a gut feeling?  Or real danger
  of some mishap?  Earlier you said "judgement call".  Not strong enough
  for me.
  
* I don't think that teaching sources about projects is a good thing.
  Existing sources don't know anything about that now and that doesn't
  stop them from contributing data to the list of
  flymake-show-project-diagnostics.  From the start, sources are defined
  to be concerned with syntax checking _one_ buffer.  99% percent of
  Flymake backends (and probably also Flycheck backends) do this.  I
  don't want to change them.  For the 1% that happen to have easy access
  to some kind of information about other files, there are the new
  options of "foreign" and "list-only" diags, depending on how this
  extra info is acquired.

Best regards,
João