--- Begin Message ---
|
Subject: |
28.0.50; Support project-wide diagnostics reports in flymake.el |
|
Date: |
Sun, 29 Aug 2021 01:53:08 +0100 |
This is coming from:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34418
https://github.com/joaotavora/eglot/pull/640
In the first, Philip wonders why the BUFFER argument to
flymake-make-diagnostic is mostly ignored.
In the second, Theodor requests that the Eglot LSP client manage and
display the diagnostics reported by the LSP server not only for the
current buffer being edited, but for other buffers or files.
The connection between the two will hopefully become apparent soon.
Normally, diagnostics reports are emitted by diagnostic backend for a
single file, usually as a result of a change performed shortly before in
the buffer visiting that file. This is true for most LSP servers when
acting as Flymake backends. But some LSP servers seem to take this
opportunity to also "piggy-back" diagnostics for other files, in a kind
of "side-band signal". Other servers yet will sometimes report
diagnostics upfront for all files in a given project, whether visited by
a buffer or not.
Theodor's pull request handles this side band signal for LSP specifically
in the Eglot client, but it contains some twists and turns that should
not be taken in Eglot. This functionality should be supported in
Flymake.
* What kind of new API support for Flymake backends is needed for this?
This is the good news: I think not much.
The helper function flymake-make-diagnostic, used by Flymake backends
such as Eglot's, already supports a BUFFER first argument, but, as the
manual states.
Currently, it is unspecified behavior to make diagnostics for
buffers other than the buffer that the Flymake backend is
responsible for.
My idea is to make it specified. As floated in bug#34418 I think the
idea of generalizing the argument to mean BUFFER-OR-FILE is decent and
sufficient.
* What kind of infrastructure changes in Flymake are needed for this?
That's more complicated: substantial, but not insane. Here's a
top-level plan: the current buffer-local variable
flymake--backend-state should probably first become a global hash
table indexed by buffer, which should be mostly indistinguishable from
a buffer-local var.
If that works, then it should be possible to store the piggy-backed
diagnostics in (1) file-indexed entries to that global hash table, for
files which are not being visited by buffers. It should also be
possible to add (2) buffer-indexed entries to it for buffers which,
though live, don't have Flymake properly activated or don't have that
specific piggy-backing backend in their running backend list.
These new types of entries will likely not contain the full-blown
flymake--backend-state structs, i.e. they will be missing the
'running', 'reported-p' and 'disabled' slots. All they will have is a
non-nil list 'diags'.
If, for situations (1) or (2), we discover that a "proper"
buffer-indexed entry already exists in the global hash table, we must
decide if the information in them should take priority. I'm not sure
of the answer to this yet.
Anyway, in flymake--handle-report is where the recording of
information should happen. We can have a look at the current
aforementioned "unspecified" behavior:
(setq new-diags
(cl-remove-if-not
(lambda (diag) (eq (flymake--diag-buffer diag) (current-buffer)))
report-action))
(As we can see, the current "unspecification" is just ignoring reports
for BUFFERs other than the current.)
* In what new UI parts is the augmented information to be useful?
Currently, I see only one place, the diagnostic listing buffer
obtained by M-x flymake-show-diagnostics-buffer. That buffer is
usually associated with only one source buffer
(flymake--diagnostics-buffer-source). Now it should start listing all
the diagnostics for buffers or files known to belong to the same
project, using 'project.el' functionality for that.
João
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#50244: 28.0.50; Support project-wide diagnostics reports in flymake.el |
|
Date: |
Tue, 14 Sep 2021 12:34:42 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Hi Theodor,
I've now pushed this to master, so I'm marking the bug done. But we
can keep discussing the Eglot patch here (or in another bug, or in
Eglot's tracker, as you prefer...)
Theodor Thornhill <theo@thornhill.no> writes:
>>Thanks. I think I'll try rust-analyzer as I've been meaning to anyway.
>>Perhaps make it the default Rust server for Eglot.
> Absolutely, the community seems to have moved on.
Actually I did still try with `rls` since that was most handy and it
also provides diagnostics for other files on startup. So I tested it
with rls and got decent results (see Eglot patch).
> By the way- these list only diagnostics do in fact happen on each
> keystroke in the mentioned servers. Not just on the first load.
> Would this mean the foreign variant is more correct to use?
Yes, it would, indeed. I don't know if _every_ server operates like
that, though. But doesn't matter. Are you saying that in your server,
making a modification to any file always brings in diagnostics for all
other files, too?
Anyway, I've tested a bit with Eglot and flymake-list-only-diagnostics
and it seems to do the right thing (mostly). I attach the Eglot patch
which you can use as starting point.
diff --git a/eglot.el b/eglot.el
index 7ebc422..d036a75 100644
--- a/eglot.el
+++ b/eglot.el
@@ -1386,6 +1386,11 @@ Doubles as an indicator of snippet support."
(font-lock-ensure)
(string-trim (filter-buffer-substring (point-min) (point-max))))))
+(defun eglot--diag-type (severity)
+ (cond ((<= severity 1) 'eglot-error)
+ ((= severity 2) 'eglot-warning)
+ (t 'eglot-note)))
+
(define-obsolete-variable-alias 'eglot-ignored-server-capabilites
'eglot-ignored-server-capabilities "1.8")
@@ -1788,8 +1793,7 @@ COMMAND is a symbol naming the command."
diag-spec
(setq message (concat source ": " message))
(pcase-let
- ((sev severity)
- (`(,beg . ,end) (eglot--range-region range)))
+ ((`(,beg . ,end) (eglot--range-region range)))
;; Fallback to `flymake-diag-region' if server
;; botched the range
(when (= beg end)
@@ -1808,16 +1812,26 @@ COMMAND is a symbol naming the command."
(point-at-eol
(1+ (plist-get (plist-get range :end)
:line)))))))
(eglot--make-diag (current-buffer) beg end
- (cond ((<= sev 1) 'eglot-error)
- ((= sev 2) 'eglot-warning)
- (t 'eglot-note))
+ (eglot--diag-type severity)
message `((eglot-lsp-diag .
,diag-spec)))))
into diags
finally (cond (eglot--current-flymake-report-fn
(eglot--report-to-flymake diags))
(t
(setq eglot--unreported-diagnostics (cons t diags))))))
- (jsonrpc--debug server "Diagnostics received for unvisited %s" uri)))
+ (cl-loop
+ with path = (expand-file-name (eglot--uri-to-path uri))
+ for diag-spec across diagnostics
+ collect (eglot--dbind ((Diagnostic) message severity source) diag-spec
+ (setq message (concat source ": " message))
+ (eglot--make-diag path (cons 1 1) nil ; cons 1 1 bcs lazy...
+ (eglot--diag-type severity)
+ message))
+ into diags
+ finally
+ (setq flymake-list-only-diagnostics
+ (assoc-delete-all path flymake-list-only-diagnostics #'string=))
+ (push (cons path diags) flymake-list-only-diagnostics))))
(cl-defun eglot--register-unregister (server things how)
"Helper for `registerCapability'.
--- End Message ---