Subject: Re: bug#64428: [PATCH] Fix flymake mode line scrolling with pixel-scroll-precision-mode

From: [email-masked]
Date: Fri, 07 Jul 2023 12:12:32 +0000 (UTC)

João Távora <[email-masked]> writes:

> On second thought, here are some comments that I think should be
> improved in Spencer's patch:
>
>> @@ -1479,21 +1505,8 @@ flymake--mode-line-counter
>>                               ((eq type :warning) "warnings")
>>                               ((eq type :note) "notes")
>>                               (t (format "%s diagnostics" type))))
>> -         keymap
>> -         ,(let ((map (make-sparse-keymap)))
>> -            (define-key map (vector 'mode-line
>> -                                    mouse-wheel-down-event)
>> -              (lambda (event)
>> -                (interactive "e")
>> -                (with-selected-window (posn-window (event-start event))
>> -                  (flymake-goto-prev-error 1 (list type) t))))
>> -            (define-key map (vector 'mode-line
>> -                                    mouse-wheel-up-event)
>> -              (lambda (event)
>> -                (interactive "e")
>> -                (with-selected-window (posn-window (event-start event))
>> -                  (flymake-goto-next-error 1 (list type) t))))
>> -            map))))))
>> +         type ,type
>
> Spencer, here you are recording the value of the `type` in a `type`
> text-property of the affected text.  Generally, though this rule
> isn't enforced or always followed (at least by me), it's better
> to give these package-specific properties some longer
> package-specific name like `flymake--diagnostic-type`.  This will
> prevent any clashes if the less-qualified `type` is ever defined
> to mean something else as a text-property.
>
>> +  (interactive "e")
>> +  (let* ((posn-string (posn-string (event-start event)))
>> +         (type (get-text-property (cdr posn-string) 'type (car posn-string))))
>> +    (with-selected-window (posn-window (event-start event))
>> +      (flymake-goto-prev-error 1 (list type) t))))
>
> And here, you could consider saving the value of (event-start event)
> by adding another early binding to that `let*`, maybe call it `estart`.
> This is much less important than the first comment though.
>
> João

Fixed.

I have tested in both graphical and tty Emacs.

>From c369c3b99ef1e5f9eab29f99a9d4f354352ef05b Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Sun, 2 Jul 2023 17:49:23 -0400
Subject: [PATCH] Fix flymake mode line scrolling with
 pixel-scroll-precision-mode

When pixel-scroll-precision-mode is enabled, scrolling the mouse wheel
will yield wheel-{up,down} events.  flymake now binds the new events
in addition to the old mouse-wheel-{up,down}-event.

* lisp/progmodes/flymake.el:(flymake--mode-line-counter-scroll-prev,
flymake--mode-line-counter-scroll-next,
flymake--mode-line-counter-map): Add.
(flymake--mode-line-counter): Use new keymap and include
flymake--diagnostic-type as a property in the mode-line.
---
 lisp/progmodes/flymake.el | 47 ++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 47dc32f9245..17154b646d7 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1449,6 +1449,36 @@ flymake--mode-line-exception
 (defun flymake--mode-line-counters ()
   (when (flymake-running-backends) flymake-mode-line-counter-format))
 
+(defun flymake--mode-line-counter-scroll-prev (event)
+  (interactive "e")
+  (let* ((event-start (event-start event))
+         (posn-string (posn-string event-start))
+         (type (get-text-property
+                (cdr posn-string) 'flymake--diagnostic-type (car posn-string))))
+    (with-selected-window (posn-window event-start)
+      (flymake-goto-prev-error 1 (list type) t))))
+
+(defun flymake--mode-line-counter-scroll-next (event)
+  (interactive "e")
+  (let* ((event-start (event-start event))
+         (posn-string (posn-string event-start))
+         (type (get-text-property
+                (cdr posn-string) 'flymake--diagnostic-type (car posn-string))))
+    (with-selected-window (posn-window event-start)
+      (flymake-goto-next-error 1 (list type) t))))
+
+(defvar flymake--mode-line-counter-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map (vector 'mode-line mouse-wheel-down-event)
+                #'flymake--mode-line-counter-scroll-prev)
+    (define-key map [mode-line wheel-down]
+                #'flymake--mode-line-counter-scroll-prev)
+    (define-key map (vector 'mode-line mouse-wheel-up-event)
+                #'flymake--mode-line-counter-scroll-next)
+    (define-key map [mode-line wheel-up]
+                #'flymake--mode-line-counter-scroll-next)
+    map))
+
 (defun flymake--mode-line-counter (type &optional no-space)
   "Compute number of diagnostics in buffer with TYPE's severity.
 TYPE is usually keyword `:error', `:warning' or `:note'."
@@ -1479,21 +1509,8 @@ flymake--mode-line-counter
                              ((eq type :warning) "warnings")
                              ((eq type :note) "notes")
                              (t (format "%s diagnostics" type))))
-         keymap
-         ,(let ((map (make-sparse-keymap)))
-            (define-key map (vector 'mode-line
-                                    mouse-wheel-down-event)
-              (lambda (event)
-                (interactive "e")
-                (with-selected-window (posn-window (event-start event))
-                  (flymake-goto-prev-error 1 (list type) t))))
-            (define-key map (vector 'mode-line
-                                    mouse-wheel-up-event)
-              (lambda (event)
-                (interactive "e")
-                (with-selected-window (posn-window (event-start event))
-                  (flymake-goto-next-error 1 (list type) t))))
-            map))))))
+         flymake--diagnostic-type ,type
+         keymap ,flymake--mode-line-counter-map)))))
 
 ;;; Per-buffer diagnostic listing
 
-- 
2.41.0