Re: Flymake refactored

[Mark Oteiza]

On 08/10/17 at 10:06am, João Távora wrote:
> Mark Oteiza <address@hidden> writes:
> 
> > On 07/10/17 at 05:07pm, João Távora wrote:
> >> Mark Oteiza <address@hidden> writes:
> >> 
> >> > Sure, I've been reading flycheck and syntastic (analogous package for
> >> > vim) for reference.
> >> >
> >> > There are some things aside from checkers I think flymake should learn
> >> > from flycheck--may as well list some here:
> >> > [...]
> >> > - popup a special buffer with all the error/warning/info listed
> >> 
> >> Please have a look at the scratch/flymake-diagnostics-buffer branch and
> >> tell me what you think (perhaps comparing it to Flycheck's). The command
> >> is flymake-show-diagnostics-buffer.
> >> 
> >> It's very naively implemented for now (and seems kinda slow).
> >
> > Looks good, I'd just change from using buttons to having the whole line
> > be usable to navigate to the error.
> 
> Agree, makes sense. But this seems akward to do in
> tabulated-list-mode. I don't mind if you beat me to it :-)

Patch below :)

> > I suspect it's the use of overlays making it slow--I don't think you
> > need overlays at all for this--just store what you need in the
> > tabulated-list id which IIRC gets applied to the whole line as a text
> > property, which you can then use with (tabulated-list-get-id)
> 
> But it doesn't make any new overlays, if that was your idea. The
> overlays used are the ones in the source buffer, where they can hardly
> be avoided. Using them here seemed like the easiest and fastest way to
> get to all Flymake diagnostics in a buffer.

Oh true, my mistake.

> I might have exaggerated the performance hit, since it doesn't seem so
> slow to me now. Perhaps we could get some big files full of errors and
> run some benchmarks with a proper backend that can be found in Flycheck,
> too.

Probably overkill, but my best example is the Freefem++ manual: an 860
kB, 21k-line .tex file which triggers thousands of warnings in
chktex(1).

It took about ten minutes for flycheck to check it (vs a few seconds to
make the diagnostic list), and you have to set
flycheck-checker-error-threshold to nil in order to prevent flycheck
from disabling the checker.  This is one thing I _don't_ like about
flycheck--I would like the ability to at least truncate results.

It doesn't look like they have a browsable repository.  You'll find it
as DOC/freefem++doc.tex in the latest tarball.
http://www.freefem.org/ff++/ftp/freefem++-3.56-1.tar.gz

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 24b1950c1a..fb5fc7db12 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -988,17 +988,19 @@ flymake--mode-line-format
 
 (defvar-local flymake--diagnostics-buffer-source nil)
 
-(defvar flymake--diagnostics-buffer-button-keymap
+(defvar flymake-diagnostics-buffer-mode-map
   (let ((map (make-sparse-keymap)))
-    (define-key map [mouse-1] 'push-button)
-    (define-key map (kbd "RET") 'push-button)
+    (define-key map [mouse-1] 'flymake-goto-diagnostic-at-point)
+    (define-key map (kbd "RET") 'flymake-goto-diagnostic-at-point)
     (define-key map (kbd "SPC") 'flymake-show-diagnostic-at-point)
     map))
 
-(defun flymake-show-diagnostic-at-point (button)
-  "Show location of diagnostic of BUTTON."
-  (interactive (list (button-at (point))))
-  (let* ((overlay (button-get button 'flymake-overlay)))
+(defun flymake-show-diagnostic-at-point ()
+  "Show location of diagnostic at point."
+  (interactive)
+  (let* ((id (or (tabulated-list-get-id)
+                 (user-error "Nothing at point")))
+         (overlay (plist-get id :overlay)))
     (with-current-buffer (overlay-buffer overlay)
       (with-selected-window
           (display-buffer (current-buffer))
@@ -1008,11 +1010,11 @@ flymake-show-diagnostic-at-point
                                           'highlight))
       (current-buffer))))
 
-(defun flymake-goto-diagnostic-at-point (button)
-  "Show location of diagnostic of BUTTON."
-  (interactive (list (button-at (point))))
+(defun flymake-goto-diagnostic-at-point ()
+  "Show location of diagnostic at point."
+  (interactive)
   (pop-to-buffer
-   (flymake-show-diagnostic-at-point button)))
+   (flymake-show-diagnostic-at-point)))
 
 (defun flymake--diagnostics-buffer-entries ()
   (with-current-buffer flymake--diagnostics-buffer-source
@@ -1032,16 +1034,7 @@ flymake--diagnostics-buffer-entries
                          :severity (flymake--lookup-type-property
                                     type
                                     'severity (warning-numeric-level :error)))
-                   `[(,(format "%s" line)
-                      keymap ,flymake--diagnostics-buffer-button-keymap
-                      action flymake-goto-diagnostic-at-point
-                      mouse-action flymake-goto-diagnostic-at-point
-                      help-echo ,(mapconcat #'identity
-                                            '("mouse-1, RET: goto location at 
point"
-                                              "SPC: show location at point")
-                                            "\n")
-                      flymake-diagnostic ,diag
-                      flymake-overlay ,ov)
+                   `[,(format "%s" line)
                      ,(format "%s" col)
                      ,(propertize (format "%s" type)
                                   'face (flymake--lookup-type-property