procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Flymake: more ambitious cleanup in flymake-mode (bug#69809)

Browse source 
Further improve flymake-mode idempotency by not nuke existing overlays.
This means multiple flymake-mode invocations do the same as just one
one, with minimal or no additional side effects.  This is good for
people with lots of 'flymake-mode' in hooks.

The foreign diagnostic importation has been refactored into a separate
function and moved to the "really start" section of 'flymake-start'.
The duplication problem appears to be avoided by some heuristics in
flymake-highlight-line.

A new test has been added.

* lisp/progmodes/flymake.el (flymake--import-foreign-diagnostics): New helper
  (flymake-start): Use it.
  (flymake-mode): Don't nuke overlays here.

* test/lisp/progmodes/flymake-tests.el (foreign-diagnostics): New
  test.
This commit is contained in:
João Távora 2024-07-18 01:09:10 +01:00
parent 3a1b36a39d
commit 0ff82eb487
2 changed files with 54 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
lisp/progmodes/flymake.el
View file

@ -1262,6 +1262,37 @@ with a report function."
"Recent changes collected by `flymake-after-change-function'.")
(defvar flymake-mode)
(defun flymake--import-foreign-diagnostics ()
;; Other diagnostic sources may already target this buffer's file
;; before we turned on: these sources may be of two types...
(let ((source (current-buffer))
(bfn buffer-file-name))
;; 1. For `flymake-list-only-diagnostics': here, we do nothing.
;; FIXME: We could remove the corresponding entry from that
;; variable, as we assume that new diagnostics will come in soon
;; via the brand new `flymake-mode' setup. For simplicity's
;; sake, we have opted to leave the backend for now.
nil
;; 2. other buffers where a backend has created "foreign
;; diagnostics" and pointed them here. We must highlight them in
;; this buffer, i.e. create overlays for them. Those other
;; buffers and backends are still responsible for them, i.e. the
;; current buffer does not "own" these foreign diags.
(dolist (buffer (buffer-list))
(with-current-buffer buffer
(when flymake-mode
(maphash (lambda (_backend state)
(maphash (lambda (file diags)
(when (or (eq file source)
(string= bfn (expand-file-name file)))
(with-current-buffer source
(mapc (lambda (diag)
(flymake--highlight-line diag
'foreign))
diags))))
(flymake--state-foreign-diags state)))
flymake--state))))))
(defun flymake-start (&optional deferred force)
"Start a syntax check for the current buffer.
DEFERRED is a list of symbols designating conditions to wait for
@ -1335,7 +1366,8 @@ Interactively, with a prefix arg, FORCE is t."
backend))
(t
(flymake--run-backend backend backend-args)))
nil))))))))
nil)))
(flymake--import-foreign-diagnostics))))))
(defvar flymake-mode-map
(let ((map (make-sparse-keymap)))
@ -1401,49 +1433,8 @@ special *Flymake log* buffer." :group 'flymake :lighter
;; already active. I.e. `flymake-mode' function should be as
;; idempotent as possible. See bug#69809.
(unless flymake--state (setq flymake--state (make-hash-table)))
;; On a related note to bug#69809, deleting all Flymake overlays is
;; a violation of that idempotence. This could be addressed in the
;; future. However, there is at least one known reason for doing so
;; currently: since "foreign diagnostics" are created here, we opt
;; to delete everything to avoid duplicating overlays. In
;; principle, the next `flymake-start' should re-synch everything
;; (and with high likelyhood that is right around the corner if
;; `flymake-start-on-flymake-mode' is t).
(mapc #'flymake--delete-overlay (flymake--really-all-overlays))
(setq flymake--recent-changes nil)
(when flymake-start-on-flymake-mode (flymake-start t))
;; Other diagnostic sources may already target this buffer's file
;; before we turned on: these sources may be of two types...
(let ((source (current-buffer))
(bfn buffer-file-name))
;; 1. For `flymake-list-only-diagnostics': here, we do nothing.
;; FIXME: We could remove the corresponding entry from that
;; variable, as we assume that new diagnostics will come in soon
;; via the brand new `flymake-mode' setup. For simplicity's
;; sake, we have opted to leave the backend for now.
nil
;; 2. other buffers where a backend has created "foreign
;; diagnostics" and pointed them here. We must highlight them in
;; this buffer, i.e. create overlays for them. Those other
;; buffers and backends are still responsible for them, i.e. the
;; current buffer does not "own" these foreign diags.
(dolist (buffer (buffer-list))
(with-current-buffer buffer
(when flymake-mode
(maphash (lambda (_backend state)
(maphash (lambda (file diags)
(when (or (eq file source)
(string= bfn (expand-file-name file)))
(with-current-buffer source
(mapc (lambda (diag)
(flymake--highlight-line diag
'foreign))
diags))))
(flymake--state-foreign-diags state)))
flymake--state))))))
(when flymake-start-on-flymake-mode (flymake-start t)))
;; Turning the mode OFF.
(t

20
test/lisp/progmodes/flymake-tests.el
View file

@ -183,6 +183,26 @@ SEVERITY-PREDICATE is used to setup
("no-problems.h")
(should-error (flymake-goto-next-error nil nil t)))))
(ert-deftest foreign-diagnostics ()
"Test Flymake in one file impacts another"
(skip-unless (and (executable-find "gcc")
(not (ert-gcc-is-clang-p))
(executable-find "make")))
(flymake-tests--with-flymake
("another-problematic-file.c")
(flymake-tests--with-flymake
("some-problems.h")
(search-forward "frob")
(backward-char 1)
(should (eq 'flymake-note (face-at-point)))
(let ((diags (flymake-diagnostics (point))))
(should (= 1 (length diags)))
(should (eq :note (flymake-diagnostic-type (car diags))))
;; This note would never been here if it werent' a foreign
;; diagnostic sourced in 'another-problematic-file.c'.
(should (string-match "previous declaration"
(flymake-diagnostic-text (car diags))))))))
(defmacro flymake-tests--assert-set (set
should
should-not)