Package: emacs;
Reported by: matthewktromp <at> gmail.com
Date: Mon, 14 Apr 2025 21:08:02 UTC
Severity: normal
Tags: patch
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #11 received at 77809 <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 77809 <at> debbugs.gnu.org, matthewktromp <at> gmail.com Subject: Re: bug#77809: adding next-error support for flymake diagnostics buffers Date: Tue, 15 Apr 2025 11:36:30 -0400
Thank you for this long-awaited contribution!
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: matthewktromp <at> gmail.com
>> Date: Mon, 14 Apr 2025 17:06:21 -0400
>>
>> Tags: patch
>>
>> This patch adds next-error support for flymake diagnostics buffers.
>> Buffers created with `flymake-show-buffer-diagnostics' and
>> `flymake-show-project-diagnostics' are now next-error enabled, and
>> `next-error' and `previous-error' will navigate through their listed
>> diagnostics.
>
> Many buffers which use Flymake already support next-error, don't they?
> Won't this conflict with those cases?
No, I think this is just giving the diagnostic overview buffers a
next-error-function, so that when you use those buffers they will
integrate with next-error.
>> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
>> index 6cc7e1f7a79..002e587b029 100644
>> --- a/lisp/progmodes/flymake.el
>> +++ b/lisp/progmodes/flymake.el
>> @@ -1390,6 +1390,10 @@ flymake-mode-map
>> (format "<%s> <mouse-1>" flymake-fringe-indicator-position)
>> #'flymake-show-buffer-diagnostics-at-event-line)
>>
>> +(defvar-local flymake-current-diagnostic-line 0
>> + "The line of the most recently focused diagnostic in the flymake
>> +diagnostics buffer.")
>> +
>> ;;;###autoload
>> (define-minor-mode flymake-mode
>> "Toggle Flymake mode on or off.
>> @@ -1881,7 +1885,8 @@ flymake-pulse-momentary-highlight-region
>> (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)
>> + (let* ((diagnostics-buffer (current-buffer))
>> + (id (or (tabulated-list-get-id pos)
>> (user-error "Nothing at point")))
>> (diag (plist-get id :diagnostic))
>> (locus (flymake--diag-locus diag))
>> @@ -1890,6 +1895,7 @@ flymake-show-diagnostic
>> (visit (lambda (b e)
>> (goto-char b)
>> (flymake-pulse-momentary-highlight-region b e))))
>> + (setq flymake-current-diagnostic-line (line-number-at-pos pos))
>> (with-current-buffer (cond ((bufferp locus) locus)
>> (t (find-file-noselect locus)))
>> (with-selected-window
>> @@ -1905,6 +1911,8 @@ flymake-show-diagnostic
>> (car beg)
>> (cdr beg))))
>> (funcall visit bbeg bend)))))
>> + (setq next-error-buffer diagnostics-buffer
>> + next-error-last-buffer diagnostics-buffer)
Setting next-error-last-buffer makes sense and matches other buffers
which set next-error-function (e.g. xref-goto-xref), but do we need to
set next-error-buffer? It seems like setting that is managed by
next-error itself.
>> (current-buffer))))
>>
>> (defun flymake-goto-diagnostic (pos)
>> @@ -2008,6 +2016,23 @@ flymake--diagnostics-base-tabulated-list-format
>> ("Backend" 8 t)
>> ("Message" 0 t)])
>>
>> +(defun flymake--diagnostics-next-error (n &optional reset)
>> + "`next-error-function' for flymake diagnostics buffers.
>> +N is an integer representing how many errors to move.
>> +If RESET is non-nil, returns to the beginning of the errors before
>> +moving."
>> + (let ((line (if reset 1 flymake-current-diagnostic-line))
>> + (total-lines (count-lines (point-min) (point-max))))
>> + (goto-char (point-min))
>> + (unless (zerop total-lines)
>> + (let ((target-line (+ line n)))
>> + (setq target-line (max 1 target-line))
>> + (setq target-line (min target-line total-lines))
>> + (forward-line (1- target-line))))
>> + (when-let ((win (get-buffer-window nil t)))
>> + (set-window-point win (point)))
>> + (flymake-goto-diagnostic (point))))
>> +
>> (define-derived-mode flymake-diagnostics-buffer-mode tabulated-list-mode
>> "Flymake diagnostics"
>> "A mode for listing Flymake diagnostics."
>> @@ -2015,6 +2040,9 @@ flymake-diagnostics-buffer-mode
>> (setq tabulated-list-format flymake--diagnostics-base-tabulated-list-format)
>> (setq tabulated-list-entries
>> 'flymake--diagnostics-buffer-entries)
>> +
>> + (setq-local next-error-function #'flymake--diagnostics-next-error)
>> +
This should be set in the major mode initialization (the body of the
define-derived-mode).
>> (tabulated-list-init-header))
>>
>> (defun flymake--diagnostics-buffer-name ()
>> @@ -2036,6 +2064,7 @@ flymake-show-buffer-diagnostics
>> (current-buffer)))))
>> (with-current-buffer target
>> (setq flymake--diagnostics-buffer-source source)
>> + (setq next-error-last-buffer (current-buffer))
>> (revert-buffer)
>> (display-buffer (current-buffer)
>> `((display-buffer-reuse-window
>> @@ -2085,6 +2114,9 @@ flymake-project-diagnostics-mode
>> (setq tabulated-list-format
>> (vconcat [("File" 25 t)]
>> flymake--diagnostics-base-tabulated-list-format))
>> +
>> + (setq-local next-error-function #'flymake--diagnostics-next-error)
>> +
Likewise, this should be set in the major mode initialization (the body
of the define-derived-mode).
>> (setq tabulated-list-entries
>> 'flymake--project-diagnostics-entries)
>> (tabulated-list-init-header))
>> @@ -2149,6 +2181,7 @@ flymake-show-project-diagnostics
>> (with-current-buffer buffer
>> (flymake-project-diagnostics-mode)
>> (setq-local flymake--project-diagnostic-list-project prj)
>> + (setq next-error-last-buffer (current-buffer))
>> (revert-buffer)
>> (display-buffer (current-buffer)
>> `((display-buffer-reuse-window
>>
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.