Re: New Flymake rewrite in emacs-26

[joaotavora@gmail.com (João Távora)]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, I've proofread it and made several fixes, please take a look.
> (I can explain each change I made, if it is not immediately apparent
> from the change itself or the log message.)

No need, looks very good, thanks.

>> > This should be in NEWS, perhaps after adjusting the style a bit.
>> 
>> I'm not great at summarizing changes in a NEWS-snazzy manner, but a
>> patch is attached.
>
> Looks good, see a couple of minor comments below.
>
>> Apart from that, I see these loose ends:
>> 
>> * Are we keeping track of the Flymake backends developed by people?
>
> You mean, in the manual?  I'd prefer not to, since such information
> quickly becomes obsolete, and keeping it up to date imposes a
> maintenance burden we'd better avoided.
>
> If you mean track them somewhere else, please tell.

No, I meant generally keep track of the backends that surfaced in this
list and merge them into the relevant major-mode in emacs-26 (in keeping
with your decision that they should be merged if they work reasonably
well).

>> * Should Flymake do something with next-error-function?
>
> I thought it already did?

It doesn't. And I should have said 'next-error' more generally. IIUC the
place for next-error-function is for major modes, which flymake-mode
isn't (but its proposed diagnostics buffer is).

Anyway I think the problem is that next-error will have a hard time (if
it doesn't already) choosing between its "next-error" source: the
compilation, and grep occur buffers, and now the constantly updated list
of Flymake annotations.

>> * There is a "Flymake diagnostics buffer" sub-feature in
>>   scratch/flymake-diagnostics-buffer.  It is reasonably stable.  Is it
>>   OK to merge into emacs-26?
>
> If it's easy to show a diff for such a merge, please do.

Patch is attached (though I don't really understand if you want to see
the diff or rather ensure that a diff is possible and easy to revert if
problems arise)

> It doesn't seem worth the hassle.  Most users will be programmers
> anyway.

Possibly not elisp programmers, but OK.

> Otherwise, please push these [NEWS] changes, and thanks.

Done.

João

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 4c4d6aef32..6796fc2b76 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -605,7 +605,12 @@ flymake-is-running
             (flymake-log :debug "backend %s reported %d diagnostics in %.2f second(s)"
                          backend
                          (length new-diags)
-                         (- (float-time) flymake-check-start-time)))))))))
+                         (- (float-time) flymake-check-start-time)))
+          (when (and (get-buffer (flymake--diagnostics-buffer-name))
+                     (get-buffer-window (flymake--diagnostics-buffer-name))
+                     (null (cl-set-difference (flymake-running-backends)
+                                              (flymake-reporting-backends))))
+            (flymake-show-diagnostics-buffer))))))))
 
 (defun flymake-make-report-fn (backend &optional token)
   "Make a suitable anonymous report function for BACKEND.
@@ -869,6 +874,7 @@ flymake-goto-prev-error
     [ "Go to previous error"  flymake-goto-prev-error t ]
     [ "Check now"             flymake-start t ]
     [ "Go to log buffer"      flymake-switch-to-log-buffer t ]
+    [ "Show error buffer"     flymake-show-diagnostics-buffer t ]
     "--"
     [ "Turn off Flymake"      flymake-mode t ]))
 
@@ -977,6 +983,102 @@ flymake--mode-line-format
                         '(:propertize " "))
              (:propertize "]")))))))
 
+
+;;; Diagnostics buffer
+
+(defvar-local flymake--diagnostics-buffer-source nil)
+
+(defvar flymake-diagnostics-buffer-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map (kbd "RET") 'flymake-goto-diagnostic)
+    (define-key map (kbd "SPC") 'flymake-show-diagnostic)
+    map))
+
+(defun flymake-show-diagnostic (pos &optional other-window)
+  "Show location of diagnostic at POS."
+  (interactive (list (point) t))
+  (let* ((id (or (tabulated-list-get-id pos)
+                 (user-error "Nothing at point")))
+         (overlay (plist-get id :overlay)))
+    (with-current-buffer (overlay-buffer overlay)
+      (with-selected-window
+          (display-buffer (current-buffer) other-window)
+        (goto-char (overlay-start overlay))
+        (pulse-momentary-highlight-region (overlay-start overlay)
+                                          (overlay-end overlay)
+                                          'highlight))
+      (current-buffer))))
+
+(defun flymake-goto-diagnostic (pos)
+  "Show location of diagnostic at POS.
+POS can be a buffer position or a button"
+  (interactive "d")
+  (pop-to-buffer
+   (flymake-show-diagnostic (if (button-type pos) (button-start pos) pos))))
+
+(defun flymake--diagnostics-buffer-entries ()
+  (with-current-buffer flymake--diagnostics-buffer-source
+    (cl-loop for ov in (flymake--overlays)
+             for diag = (overlay-get ov
+                                     'flymake--diagnostic)
+             for (line . col) =
+             (save-excursion
+               (goto-char (overlay-start ov))
+               (cons (line-number-at-pos)
+                     (- (point)
+                        (line-beginning-position))))
+             for type = (flymake--diag-type diag)
+             collect
+             (list (list :overlay ov
+                         :line line
+                         :severity (flymake--lookup-type-property
+                                    type
+                                    'severity (warning-numeric-level :error)))
+                   `[,(format "%s" line)
+                     ,(format "%s" col)
+                     ,(propertize (format "%s" type)
+                                  'face (flymake--lookup-type-property
+                                         type 'mode-line-face 'flymake-error))
+                     (,(format "%s" (flymake--diag-text diag))
+                      mouse-face highlight
+                      help-echo "mouse-2: visit this diagnostic"
+                      face nil
+                      mouse-action flymake-goto-diagnostic)]))))
+
+(define-derived-mode flymake-diagnostics-buffer-mode tabulated-list-mode
+  "Flymake diagnostics"
+  "A mode for listing Flymake diagnostics."
+  (setq tabulated-list-format
+        `[("Line" 5 (lambda (l1 l2)
+                      (< (plist-get (car l1) :line)
+                         (plist-get (car l2) :line)))
+           :right-align t)
+          ("Col" 3 nil :right-align t)
+          ("Type" 8 (lambda (l1 l2)
+                      (< (plist-get (car l1) :severity)
+                         (plist-get (car l2) :severity))))
+          ("Message" 0 t)])
+  (setq tabulated-list-entries
+        'flymake--diagnostics-buffer-entries)
+  (tabulated-list-init-header))
+
+(defun flymake--diagnostics-buffer-name ()
+  (format "*Flymake diagnostics for %s*" (current-buffer)))
+
+(defun flymake-show-diagnostics-buffer ()
+  "Show a list of Flymake diagnostics for current buffer."
+  (interactive)
+  (let* ((name (flymake--diagnostics-buffer-name))
+         (source (current-buffer))
+         (target (or (get-buffer name)
+                     (with-current-buffer (get-buffer-create name)
+                       (flymake-diagnostics-buffer-mode)
+                       (setq flymake--diagnostics-buffer-source source)
+                       (current-buffer)))))
+    (with-current-buffer target
+      (revert-buffer)
+      (display-buffer (current-buffer)))))
+
 (provide 'flymake)
 
 (require 'flymake-proc)