Subject: Re: [PATCH] [UPDATE3] Fix mouse click on flymake

From: Ergus <[email-masked]>
Date: Sat, 25 Jan 2025 22:26:38 +0100

Hi Juri:

On Sat, Jan 25, 2025 at 07:48:33PM +0200, Juri Linkov wrote:
>> Please find attached the patch with the fix + the new feature.
>
>Your previous patch worked at least for margins, but new one doesn't work.
>
Try it now please. I actually detected the issue just now and was
planning to submit a new patch. when I saw your message

The issue was that I was checking for errors only in the event
position. But on fringe events `pos' is always the line start position,
where not necessarily needs to be any error overlay.

Now the code considers the whole line when the event is from the fringe
or margin and uses only pos in the flymake-menu's command.

If the line has more than one error, it highlights the first one
returned by flymake-diagnostics.

>> It includes:
>
>> 1. A new defcustom equivalent to xref-after-jump-hook.
>> 2. An optional argument to flymake-show-buffer-diagnostics.
>> 3. Modification to flymake-menu to use a new function that also highlights
>> the candidate when possible.
>
>Thanks, nice idea.
>
>> They look like too many, but are actually just a few lines, so I put
>> them together.
>
>I wonder why you don't send patches to bug-gnu-emacs
>where it's easier to track them.

I actually prefer to create a branch and so on, but I thought that this
patch would be just a couple of lines (like in my first message). But
things are never as simple as I would like ;)

Best,
Ergus
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 9dda53713f5..b7a68e382ef 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -748,34 +748,41 @@ flymake--severity
   (flymake--lookup-type-property type 'severity
                                  (warning-numeric-level :error)))
 
-(defun flymake--indicator-overlay-spec (indicator)
+(defun flymake--indicator-overlay-spec (type)
   "Return INDICATOR as propertized string to use in error indicators."
-  (let* ((value (if (symbolp indicator)
+  (let* ((indicator (flymake--lookup-type-property
+                     type
+                     (cond ((eq flymake-indicator-type 'fringes)
+                            'flymake-bitmap)
+                           ((eq flymake-indicator-type 'margins)
+                            'flymake-margin-string))
+                     (alist-get 'bitmap (alist-get type ; backward compat
+                                                   flymake-diagnostic-types-alist))))
+         (value (if (symbolp indicator)
                     (symbol-value indicator)
                   indicator))
-         (indicator-car (if (listp value)
-                            (car value)
-                          value))
-         (indicator-cdr (if (listp value)
-                            (cdr value))))
+         (valuelist (if (listp value)
+                        value
+                      (list value)))
+         (indicator-car (car valuelist)))
+
     (cond
      ((and (symbolp indicator-car)
            flymake-fringe-indicator-position)
       (propertize "!" 'display
-                  (cons flymake-fringe-indicator-position
-                        (if (listp value)
-                            value
-                          (list value)))))
+                  (cons flymake-fringe-indicator-position valuelist)))
      ((and (stringp indicator-car)
            flymake-margin-indicator-position)
       (propertize "!"
                   'display
                   `((margin ,flymake-margin-indicator-position)
-                    ,(propertize
-                      indicator-car
-                      'face
-                      `(:inherit (,indicator-cdr
-                                  default)))))))))
+                    ,(propertize indicator-car
+                                 'face `(:inherit (,(cdr valuelist) default))
+                                 'mouse-face 'highlight
+                                 'help-echo "Open Flymake diagnostics"
+                                 'keymap `,(define-keymap
+                                             (format "<%s> <mouse-1>" flymake-margin-indicator-position)
+                                             #'flymake-show-buffer-diagnostics-at-event-line))))))))
 
 (defun flymake--resize-margins (&optional orig-width)
   "Resize current window margins according to `flymake-margin-indicator-position'.
@@ -941,16 +948,7 @@ flymake--highlight-line
                   (overlay-put ov prop (flymake--lookup-type-property
                                         type prop value)))))
       (default-maybe 'face 'flymake-error)
-      (default-maybe 'before-string
-        (flymake--indicator-overlay-spec
-         (flymake--lookup-type-property
-          type
-          (cond ((eq flymake-indicator-type 'fringes)
-                 'flymake-bitmap)
-                ((eq flymake-indicator-type 'margins)
-                 'flymake-margin-string))
-          (alist-get 'bitmap (alist-get type ; backward compat
-                                        flymake-diagnostic-types-alist)))))
+      (default-maybe 'before-string (flymake--indicator-overlay-spec type))
       ;; (default-maybe 'after-string
       ;;                (flymake--diag-text diagnostic))
       (default-maybe 'help-echo
@@ -1369,12 +1367,21 @@ flymake-start
                   nil)))
              (flymake--import-foreign-diagnostics))))))
 
-(defvar flymake-mode-map
-  (let ((map (make-sparse-keymap)))
-    (define-key map `[,flymake-fringe-indicator-position mouse-1]
-                #'flymake-show-buffer-diagnostics)
-    map)
-  "Keymap for `flymake-mode'.")
+(defun flymake-show-buffer-diagnostics-at-event-line (event)
+  "Show diagnostics buffer on mouse click in the margin or fringe.
+This uses two different approaches to work.
+For margin it is set as a char property of the margin character directly.
+While in the fringe it is set as part of the `flymake-mode-map'."
+  (interactive "e")
+  (when-let* ((diagnostics (flymake-diagnostics-at-mouse-event event t)))
+    (flymake-show-buffer-diagnostics (car diagnostics))))
+
+;; Set the fringe mouse-1 action directly and perform the filtering
+;; latter iterating over the overlays.
+(defvar-keymap flymake-mode-map
+               :doc "Keymap for `flymake-mode'."
+               (format "<%s> <mouse-1>" flymake-fringe-indicator-position)
+               #'flymake-show-buffer-diagnostics-at-event-line)
 
 ;;;###autoload
 (define-minor-mode flymake-mode
@@ -1616,6 +1623,27 @@ flymake-goto-prev-error
                      t))
   (flymake-goto-next-error (- (or n 1)) filter interactive))
 
+(defun flymake-diagnostics-at-mouse-event (event full-line)
+  "Get the flymake diagnostics for a position given by a mouse EVENT.
+When FULL-LINE is not nil if gives all the diagnostics in the EVENT's
+line.
+"
+  (mouse-minibuffer-check event)
+  (let* ((posn (event-end event))
+	 (pos (posn-point posn)))
+    (when (numberp pos)
+      (with-selected-window (posn-window posn)
+	(with-current-buffer (window-buffer)
+	  (goto-char pos)
+          (if full-line
+              (flymake-diagnostics (line-beginning-position) (line-end-position))
+            (flymake-diagnostics pos (1+ pos))))))))
+
+(defun flymake-show-buffer-diagnostics-at-event-position (event)
+  (interactive "e")
+  (flymake-show-buffer-diagnostics
+   (car (flymake-diagnostics-at-mouse-event event nil))))
+
 
 ;;; Mode-line and menu
 ;;;
@@ -1624,7 +1652,7 @@ flymake-menu
     [ "Go to next problem"      flymake-goto-next-error t ]
     [ "Go to previous problem"  flymake-goto-prev-error t ]
     [ "Check now"               flymake-start t ]
-    [ "List all problems"       flymake-show-buffer-diagnostics t ]
+    [ "List all problems"       flymake-show-buffer-diagnostics-at-event-position t ]
     "--"
     [ "Go to log buffer"        flymake-switch-to-log-buffer t ]
     [ "Turn off Flymake"        flymake-mode t ]))
@@ -1652,6 +1680,15 @@ flymake-mode-line-lighter
   :type 'string
   :version "29.1")
 
+(defcustom flymake-after-jump-hook '(pulse-momentary-highlight-one-line)
+  "Hook called after jumping to a diagnostic line.
+
+This hooks are called when `flymake-show-buffer-diagnostics' receives an
+the optional `diagnostic' argument and it matches an entry in the
+diagnostic's buffer."
+  :type 'hook
+  :version "31.0")
+
 (defvar flymake-mode-line-title '(:eval (flymake--mode-line-title))
   "Mode-line construct to show Flymake's mode name and menu.")
 
@@ -1969,7 +2006,7 @@ flymake--diagnostics-buffer-name
 (define-obsolete-function-alias 'flymake-show-diagnostics-buffer
   'flymake-show-buffer-diagnostics "1.2.1")
 
-(defun flymake-show-buffer-diagnostics ()
+(defun flymake-show-buffer-diagnostics (&optional diagnostic)
   "Show a list of Flymake diagnostics for current buffer."
   (interactive)
   (unless flymake-mode
@@ -1987,7 +2024,15 @@ flymake-show-buffer-diagnostics
                       `((display-buffer-reuse-window
                          display-buffer-below-selected)
                         (window-height . (lambda (window)
-                          (fit-window-to-buffer window 10))))))))
+                                           (fit-window-to-buffer window 10)))))
+      (when (and diagnostic flymake-after-jump-hook)
+        (goto-char (point-min))
+        (catch 'done
+          (while-let ((id (tabulated-list-get-id (point))))
+            (if (eq (plist-get id :diagnostic) diagnostic)
+                (progn (run-hooks 'flymake-after-jump-hook)
+                       (throw 'done nil))
+              (forward-line))))))))
 
 
 ;;; Per-project diagnostic listing
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 263b980ac18..791aeef5765 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1559,7 +1559,6 @@ project-compile-helper
 ;;;###autoload (autoload 'project-test "project")
 (project-compile-helper project-test :test-command)
 
-
 ;;;###autoload
 (defun project-recompile (&optional edit-command)
   "Run `recompile' in the project root with an appropriate buffer."