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

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

[sbaugh]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Tags: patch


This patch is based on the patch in bug#64425 (since I noticed this
unrelated issue while working on that, and they touch the same code).



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 'type as a
property in the mode-line.


In GNU Emacs 29.0.92 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-07-02 built on earth
Repository revision: c361966508e2da159b5e65c37dff7f78e87b3445
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: NixOS 23.05 (Stoat)

Configured using:
 'configure --with-x-toolkit=lucid --with-tree-sitter --with-xinput2'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-flymake-mode-line-scrolling-with-pixel-scroll-pr.patch --]
[-- Type: text/patch, Size: 3616 bytes --]

From ead790ca0525472ed52314b8bab67e7d246cda4e Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
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 'type as a
property in the mode-line.
---
 lisp/progmodes/flymake.el | 43 +++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 47dc32f9245..a424b0b6c8d 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1449,6 +1449,32 @@ 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* ((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))))
+
+(defun flymake--mode-line-counter-scroll-next (event)
+  (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-next-error 1 (list type) t))))
+
+(setq 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 +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
+         keymap ,flymake--mode-line-counter-map)))))
 
 ;;; Per-buffer diagnostic listing
 
-- 
2.41.0

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

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Sun, 02 Jul 2023 21:51:01 +0000 (UTC)
> 
> This patch is based on the patch in bug#64425 (since I noticed this
> unrelated issue while working on that, and they touch the same code).
> 
> 
> 
> 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 'type as a
> property in the mode-line.

Thanks.  Any reason you couldn't simply add more events to the
existing code?

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

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Sun, 02 Jul 2023 21:51:01 +0000 (UTC)
>> 
>> This patch is based on the patch in bug#64425 (since I noticed this
>> unrelated issue while working on that, and they touch the same code).
>> 
>> 
>> 
>> 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 'type as a
>> property in the mode-line.
>
> Thanks.  Any reason you couldn't simply add more events to the
> existing code?

Two reasons:

1. I initially did that but it made the code uglier.  Also, this code is
run every time the mode-line is updated, and adding more of that seems
bad.

2. This makes describe-key work better and makes it possible for users
to configure the scroll direction or add more bindings for different
things.

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  64428@debbugs.gnu.org
> Date: Thu, 06 Jul 2023 09:16:41 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> * 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 'type as a
> >> property in the mode-line.
> >
> > Thanks.  Any reason you couldn't simply add more events to the
> > existing code?
> 
> Two reasons:
> 
> 1. I initially did that but it made the code uglier.  Also, this code is
> run every time the mode-line is updated, and adding more of that seems
> bad.
> 
> 2. This makes describe-key work better and makes it possible for users
> to configure the scroll direction or add more bindings for different
> things.

This means your changeset is actually two different loosely-related
changes.  The one that refactors the original code needs approval from
João (CC'ed).  Once João is happy with that refactoring, I'm okay with
adding the extra events to it.

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

[João Távora]

On Thu, Jul 6, 2023 at 3:04 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Spencer Baugh <sbaugh@janestreet.com>
> > Cc: sbaugh@catern.com,  64428@debbugs.gnu.org
> > Date: Thu, 06 Jul 2023 09:16:41 -0400
> >
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> > >> * 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 'type as a
> > >> property in the mode-line.
> > >
> > > Thanks.  Any reason you couldn't simply add more events to the
> > > existing code?
> >
> > Two reasons:
> >
> > 1. I initially did that but it made the code uglier.  Also, this code is
> > run every time the mode-line is updated, and adding more of that seems
> > bad.
> >
> > 2. This makes describe-key work better and makes it possible for users
> > to configure the scroll direction or add more bindings for different
> > things.
>
> This means your changeset is actually two different loosely-related
> changes.  The one that refactors the original code needs approval from
> João (CC'ed).  Once João is happy with that refactoring, I'm okay with
> adding the extra events to it.

Hi Eli, Spencer,

I've reviewed the code.  I understand Spencer's rationale for the
small refactoring and I'm OK with it.

However I didn't test.  I'd say it's better for this new capability to
go to master, though with some minimal testing (say, at least in tty
as well as graphical mode) it shouldn't be really dangerous for
emacs-29 either.

João

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

[João Távora]

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

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

[sbaugh]

[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]

João Távora <joaotavora@gmail.com> 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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-flymake-mode-line-scrolling-with-pixel-scroll-pr.patch --]
[-- Type: text/x-patch, Size: 3747 bytes --]

From c369c3b99ef1e5f9eab29f99a9d4f354352ef05b Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
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

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

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Fri, 7 Jul 2023 12:37:44 +0100
> Cc: Spencer Baugh <sbaugh@janestreet.com>, sbaugh@catern.com, 64428@debbugs.gnu.org
> 
> I've reviewed the code.  I understand Spencer's rationale for the
> small refactoring and I'm OK with it.
> 
> However I didn't test.  I'd say it's better for this new capability to
> go to master, though with some minimal testing (say, at least in tty
> as well as graphical mode) it shouldn't be really dangerous for
> emacs-29 either.

Thanks.  It will definitely go to master, not to emacs-29.

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

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Fri, 07 Jul 2023 12:12:32 +0000 (UTC)
> Cc: Eli Zaretskii <eliz@gnu.org>, Spencer Baugh <sbaugh@janestreet.com>,
> 	64428@debbugs.gnu.org
> 
> João Távora <joaotavora@gmail.com> 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.

Thanks.  João, is this good to go, from your POV?

I admit I consider it a bit strange to have commands that are
"internal" and don't have a doc string:

> +(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))))

Any reasons not to make them public and add doc strings?

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

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 4011 bytes --]

On Thu, Jul 13, 2023, 06:56 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: sbaugh@catern.com
> > Date: Fri, 07 Jul 2023 12:12:32 +0000 (UTC)
> > Cc: Eli Zaretskii <eliz@gnu.org>, Spencer Baugh <sbaugh@janestreet.com>,
> >       64428@debbugs.gnu.org
> >
> > João Távora <joaotavora@gmail.com> 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.
>
> Thanks.  João, is this good to go, from your POV?
>
> I admit I consider it a bit strange to have commands that are
> "internal" and don't have a doc string:
>
> > +(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))))
>
> Any reasons not to make them public and add doc strings?
>

If you can give a reasonable example of sonething the general public would
want to do with them, ok.  Until then, I think public interfaces should be
kept small.

Yes, it's good for pushing.

João

>

[-- Attachment #2: Type: text/html, Size: 5903 bytes --]

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

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Thu, 13 Jul 2023 09:12:33 +0100
> Cc: sbaugh@catern.com, Spencer Baugh <sbaugh@janestreet.com>, 64428@debbugs.gnu.org
> 
>  Any reasons not to make them public and add doc strings?
> 
> If you can give a reasonable example of sonething the general public would want to do with them, ok. 
> Until then, I think public interfaces should be kept small.
> 
> Yes, it's good for pushing.

Thanks, installed on master, and closing the bug.

Spencer, please in the future try to follow closer our conventions of
formatting commit log messages.