bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Aidan Beggs]

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

I'm running into errors when attempting to run
flymake-show-diagnostics-buffer. If run after I've enabled line numbers via
global-display-line-numbers-mode, I get the following stack trace:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  flymake--diagnostics-buffer-entries()
  tabulated-list-print(t)
  tabulated-list-revert()
  run-hooks(display-line-numbers-mode-hook
display-line-numbers-mode-on-hook)
  display-line-numbers-mode()
  display-line-numbers--turn-on()
  global-display-line-numbers-mode-enable-in-buffers()
  run-hooks(after-change-major-mode-hook)
  run-mode-hooks(flymake-diagnostics-buffer-mode-hook)
  flymake-diagnostics-buffer-mode()
  flymake-show-diagnostics-buffer()
  funcall-interactively(flymake-show-diagnostics-buffer)
  call-interactively(flymake-show-diagnostics-buffer record nil)
  command-execute(flymake-show-diagnostics-buffer record)
  execute-extended-command(nil "flymake-show-diagnostics-buffer"
"flymake-show-diagnostics")
  funcall-interactively(execute-extended-command nil
"flymake-show-diagnostics-buffer" "flymake-show-diagnostics")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

Once this has been thrown once, running flymake-show-diagnostics-buffer
works properly. The only thing in my config file is
(global-display-line-numbers-mode).

To repro:

emacs -Q
C-x b *scratch*
M-x global-display-line-numbers-mode
M-x flymake-mode
M-x flymake-show-diagnostics-buffer

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Flymake mode enabled in current buffer
tabulated-list-print: Wrong type argument: stringp, nil
Making completion list...

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD LCMS2

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  flymake-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: Aidan Beggs <nadiasggeb001@gmail.com>
> Date: Thu, 9 Apr 2020 16:55:44 -0400
> 
> emacs -Q
> C-x b *scratch*
> M-x global-display-line-numbers-mode
> M-x flymake-mode
> M-x flymake-show-diagnostics-buffer

João, here are the results of my preliminary investigation into this,
as posted on Reddit (where this was first reported), I hope they will
be helpful:

  Under the above settings, flymake--diagnostics-buffer-entries is
  invoked as side effect of calling flymake-diagnostics-buffer-mode from
  flymake-show-diagnostics-buffer, and at that point
  flymake--diagnostics-buffer-source has not yet been set to a valid
  buffer name.  So the with-current-buffer form at the beginning of
  flymake--diagnostics-buffer-entries barfs.

Thanks.

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

Thanks, I will have a look at this soon.  One thing I don't understand is
the influence of `global-display-line-numbers-mode` in all of this. I
suppose
the situation doesn't happen if it's off.  Knowing why will probably solve
the mystery.

Actually, having a look at the backtrace, it seems turning on
display-line-numbers-mode causes the tabulated list to reprint itself via
some
hook that is presumably there by default.

Anyway, this afternoon or tomorrow.

João

On Fri, Apr 10, 2020 at 7:30 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Aidan Beggs <nadiasggeb001@gmail.com>
> > Date: Thu, 9 Apr 2020 16:55:44 -0400
> >
> > emacs -Q
> > C-x b *scratch*
> > M-x global-display-line-numbers-mode
> > M-x flymake-mode
> > M-x flymake-show-diagnostics-buffer
>
> João, here are the results of my preliminary investigation into this,
> as posted on Reddit (where this was first reported), I hope they will
> be helpful:
>
>   Under the above settings, flymake--diagnostics-buffer-entries is
>   invoked as side effect of calling flymake-diagnostics-buffer-mode from
>   flymake-show-diagnostics-buffer, and at that point
>   flymake--diagnostics-buffer-source has not yet been set to a valid
>   buffer name.  So the with-current-buffer form at the beginning of
>   flymake--diagnostics-buffer-entries barfs.
>
> Thanks.
>


-- 
João Távora

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Fri, 10 Apr 2020 12:50:19 +0100
> Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> 
> Thanks, I will have a look at this soon.  One thing I don't understand is 
> the influence of `global-display-line-numbers-mode` in all of this. I suppose
> the situation doesn't happen if it's off.  Knowing why will probably solve
> the mystery.
> 
> Actually, having a look at the backtrace, it seems turning on 
> display-line-numbers-mode causes the tabulated list to reprint itself via some
> hook that is presumably there by default.  

Exactly.  I think this answers your question in the first paragraph.

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Fri, Apr 10, 2020 at 1:16 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Fri, 10 Apr 2020 12:50:19 +0100
> > Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> >
> > Thanks, I will have a look at this soon.  One thing I don't understand
> is
> > the influence of `global-display-line-numbers-mode` in all of this. I
> suppose
> > the situation doesn't happen if it's off.  Knowing why will probably
> solve
> > the mystery.
> >
> > Actually, having a look at the backtrace, it seems turning on
> > display-line-numbers-mode causes the tabulated list to reprint itself
> via some
> > hook that is presumably there by default.
>
> Exactly.  I think this answers your question in the first paragraph.
>

Yes, and it raises another one for which I don't have an answer to.
Why does it do that? What do line numbers have to do with
tabulated lists?

João Távora

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Fri, 10 Apr 2020 15:38:58 +0100
> Cc: nadiasggeb001@gmail.com, 40529@debbugs.gnu.org
> 
>  Exactly.  I think this answers your question in the first paragraph.
> 
> Yes, and it raises another one for which I don't have an answer to.  
> Why does it do that? What do line numbers have to do with 
> tabulated lists?

When line numbers are turned on or off in a buffer under
tabulated-list-mode, the buffer needs to be redisplayed because the
column counts change, and that affects alignment of the columns.
That's why tabulated-list-mode defines a function to be run from
display-line-numbers-mode-hook.

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Fri, Apr 10, 2020 at 4:50 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Fri, 10 Apr 2020 15:38:58 +0100
> > Cc: nadiasggeb001@gmail.com, 40529@debbugs.gnu.org
> >
> >  Exactly.  I think this answers your question in the first paragraph.
> >
> > Yes, and it raises another one for which I don't have an answer to.
> > Why does it do that? What do line numbers have to do with
> > tabulated lists?
>
> When line numbers are turned on or off in a buffer under
> tabulated-list-mode, the buffer needs to be redisplayed because the
> column counts change, and that affects alignment of the columns.
> That's why tabulated-list-mode defines a function to be run from
> display-line-numbers-mode-hook.
>

Thanks.  That makes some sense, though I didn't know line numbers
affected column counts in the actual buffer contents (I though they used
the margin or the fringe, but I confess my ignorance here).  Anyway, this
leaves me wondering if there isn't a hook ordering bug here. Maybe it's
just a question of delaying entry in tabulated-list-mode until important
stuff
is set up.  I don't know, I haven't looked at actual code yet.

João

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Fri, Apr 10, 2020 at 5:09 PM João Távora <joaotavora@gmail.com> wrote:

> leaves me wondering if there isn't a hook ordering bug here. Maybe it's
> just a question of delaying entry in tabulated-list-mode until important
stuff
> is set up.

Or maybe all that's needed is this (100% untested) change:

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 25a2152f00..72171f5f8b 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1378,10 +1378,10 @@ flymake-show-diagnostics-buffer
          (source (current-buffer))
          (target (or (get-buffer name)
                      (with-current-buffer (get-buffer-create name)
+                       (setq flymake--diagnostics-buffer-source source)
                        (flymake-diagnostics-buffer-mode)
                        (current-buffer)))))
     (with-current-buffer target
-      (setq flymake--diagnostics-buffer-source source)
       (revert-buffer)
       (display-buffer (current-buffer)))))

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Fri, Apr 10, 2020 at 5:16 PM João Távora <joaotavora@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 5:09 PM João Távora <joaotavora@gmail.com> wrote:
>
> > leaves me wondering if there isn't a hook ordering bug here. Maybe it's
> > just a question of delaying entry in tabulated-list-mode until
important stuff
> > is set up.
>
> Or maybe all that's needed is this (100% untested) change:
>
> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el

Finally looked into the problem and no, this doesn't work because
flymake-diagnostics-buffer-mode erases local vars before setting up the
tabulated-list-mode derived mode.  This is kind of a chicken-and-egg
issue that can only be fixed in flymake.el with some ugly hacks.

But after some analysis, I think it is tabulated-list-mode, or rather
its recent adaptation to display-line-numbers-mode, which is in the
wrong here.  It used to be that using a mode derived from
tabulated-list-mode didn't immediately force a request for refreshing its
rows.  After display-line-numbers-mode came along, that ceased to be
true in some situations.

Reading the special code in tabulated-list-mode concerned with line
numbers, the latter seem to affect only the header line, not the
buffer's contents.  So this seems to be the correct fix:

commit 0145b3c87f329e51c729703d778848cdc8393a33
Author: João Távora <joaotavora@gmail.com>
Date:   Sun Apr 12 13:10:45 2020 +0100

    Fix tabulated-list-mode bootstrapping problem

    Fixes: bug#40529
    Don't refresh all the tabulated-list rows when entering
    tabulated-list-mode just because display-line-number-mode is t.

    * lisp/emacs-lisp/tabulated-list.el (tabulated-list-mode): Don't
    call tabulated-list-revert, just tabulated-list-init-header.

diff --git a/lisp/emacs-lisp/tabulated-list.el
b/lisp/emacs-lisp/tabulated-list.el
index 501cc3a29e..68eaef1fcd 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -765,7 +765,7 @@ tabulated-list-mode
   ;; column of the first entry happens to begin with a R2L letter.
   (setq bidi-paragraph-direction 'left-to-right)
   ;; This is for if/when they turn on display-line-numbers
-  (add-hook 'display-line-numbers-mode-hook #'tabulated-list-revert nil t)
+  (add-hook 'display-line-numbers-mode-hook #'tabulated-list-init-header
nil t)
   ;; This is for if/when they customize the line-number face or when
   ;; the line-number width needs to change due to scrolling.
   (setq-local tabulated-list--current-lnum-width 0)

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sun, 12 Apr 2020 13:22:34 +0100
> Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> 
> But after some analysis, I think it is tabulated-list-mode, or rather
> its recent adaptation to display-line-numbers-mode, which is in the
> wrong here.  It used to be that using a mode derived from
> tabulated-list-mode didn't immediately force a request for refreshing its 
> rows.  After display-line-numbers-mode came along, that ceased to be
> true in some situations.
> 
> Reading the special code in tabulated-list-mode concerned with line
> numbers, the latter seem to affect only the header line, not the
> buffer's contents.  So this seems to be the correct fix:

I don't remember all the callers/users of tabulated-list-mode, so I
cannot be sure your patch is correct.  (Why only update the header?
the columns below the header need to be realigned as well, no?  See
tabulated-line-print-col, for example.)

However, if you are convinced it's TRT, I'm okay with doing that on
master.  On the release branch, please try to find a solution that is
not in tabulated-list-mode, but in Flymake.

TIA

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Sun, Apr 12, 2020 at 2:43 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Sun, 12 Apr 2020 13:22:34 +0100
> > Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> >
> > But after some analysis, I think it is tabulated-list-mode, or rather
> > its recent adaptation to display-line-numbers-mode, which is in the
> > wrong here.  It used to be that using a mode derived from
> > tabulated-list-mode didn't immediately force a request for refreshing
> its
> > rows.  After display-line-numbers-mode came along, that ceased to be
> > true in some situations.
> >
> > Reading the special code in tabulated-list-mode concerned with line
> > numbers, the latter seem to affect only the header line, not the
> > buffer's contents.  So this seems to be the correct fix:
>
> I don't remember all the callers/users of tabulated-list-mode, so I
> cannot be sure your patch is correct.  (Why only update the header?
> the columns below the header need to be realigned as well, no?  See
> tabulated-line-print-col, for example.)
>

I don't know what you are calling attention to in that function. Can
you be more specific?

Regarding your question (why only the header?), well that seems
to be the only place where a line-number _isn't_ printed, so
it needs the additional indentation.  I think that also explains
why you already _only_ update only the header in pre-redisplay-functions
and window-scroll-functions.  Have a look at
tabulated-list-watch-line-number-width and
tabulated-list-window-scroll-function. All they do is update the
header, not the contents.

I'm just extending that criteria to the turn-on/off of d-l-n-m.

Furthermore I think it is a regression in itself to deliver an Emacs
27 with this changed tabulated-list-mode bootstrapping behaviour.

However, if you are convinced it's TRT, I'm okay with doing that on
> master.  On the release branch, please try to find a solution that is
> not in tabulated-list-mode, but in Flymake.
>

I tried around a bit and couldn't come up with anything that I know is
safe, short of some very ugly vapourware hacks. Not saying that it
doesn't exist, but I started searching in tabulated-list-mode and
I do think that's the best place to fix it.

João

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sun, 12 Apr 2020 15:13:35 +0100
> Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> 
>  I don't remember all the callers/users of tabulated-list-mode, so I
>  cannot be sure your patch is correct.  (Why only update the header?
>  the columns below the header need to be realigned as well, no?  See
>  tabulated-line-print-col, for example.)
> 
> I don't know what you are calling attention to in that function. Can 
> you be more specific?

AFAIR, the :align-to display spec needs to be recalculated when line
numbers are turned on or off.

> Furthermore I think it is a regression in itself to deliver an Emacs 
> 27 with this changed tabulated-list-mode bootstrapping behaviour.

Maybe so, but that code endured many months on the master branch and
then in the pretest, so we have some reason to believe it is correct.
The code was introduced to solve real problems in some users of
tabulated-list-mode (and we have quite a few of them in core alone).

>  However, if you are convinced it's TRT, I'm okay with doing that on
>  master.  On the release branch, please try to find a solution that is
>  not in tabulated-list-mode, but in Flymake.
> 
> I tried around a bit and couldn't come up with anything that I know is 
> safe, short of some very ugly vapourware hacks. Not saying that it 
> doesn't exist, but I started searching in tabulated-list-mode and 
> I do think that's the best place to fix it.

Like I said: making changes in tabulated-list-mode is too risky at
this point for the release branch.

I'm sure a simple solution for Flymake can be found.  E.g., what about
skipping the entire body of flymake--diagnostics-buffer-entries if
flymake--diagnostics-buffer-source is nil?

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[João Távora]

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

On Sun, Apr 12, 2020 at 3:42 PM Eli Zaretskii <eliz@gnu.org> wrote:

> AFAIR, the :align-to display spec needs to be recalculated when line
> numbers are turned on or off.

I don't know what that is, but if it involves propertizing
the rows themselves, there's possiblya bug when you gain
or lose a character when scrolling. As evidenced by the fact
that the hooks I showed you also only recalculate the header.

> The code was introduced to solve real problems in some users of
> tabulated-list-mode (and we have quite a few of them in core alone).

Do you have reason to believe these would resurface if you
did my change? What are these "real problems"?  Can
references to them be found in the git log?

> Maybe so, but that code endured many months on the master branch and
> then in the pretest, so we have some reason to believe it is correct.

I don't think it's correct to ask the buffer's row-providing
backend to produce rows just you turned on the mode.
Certainly I can't think how it can be correct to do that depending
on whether or not a totally unrelated customization related to
presentation is enabled or not.

If you disagree, I would at least indicate this possibility in
the manual before releasing.

The manual says, in
https://www.gnu.org/software/emacs/manual/html_node/elisp/Tabulated-List-Mode.html

   The listing command should create or switch to a
   buffer, turn on the derived mode, specify the tabulated data,
   and finally call tabulated-list-print to populate the buffer.

Maybe you should add something explaining that sometimes
populating happens automatically, sometimes not.  If gathering
the data is expensive, the client code can be doing it twice,
a bad thing IMO.

But even conceptually and intuitively, changing the line numbers
to the left of a buffer shouldn't need recalculating the buffer's
contents. Even the effect on the header line smells a little, to
be honest. Shouldn't line numbers care to handle also push
the header line forward as they do the remainder of the buffer?
I don't know and I don't use line numbers, just wondering.

> I'm sure a simple solution for Flymake can be found.  E.g., what about
> skipping the entire body of flymake--diagnostics-buffer-entries if
> flymake--diagnostics-buffer-source is nil

Maybe that works, yes. Feel free to try it and commit it
to Emacs 27, I have little time and I'm booted into a machine
with no Emacs.

Thanks,
João

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sun, 12 Apr 2020 17:58:16 +0100
> Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> 
> > I'm sure a simple solution for Flymake can be found.  E.g., what about
> > skipping the entire body of flymake--diagnostics-buffer-entries if
> > flymake--diagnostics-buffer-source is nil
> 
> Maybe that works, yes. Feel free to try it and commit it 
> to Emacs 27, I have little time and I'm booted into a machine 
> with no Emacs.

OK.

Aidan, can you try the patch below and see if it solves the problem?
The initial error in the recipe you posted is definitely gone after
applying the patch, but please also try this after that and see that
flymake-show-diagnostics-buffer also works after that, in real-life
use.

Thanks.

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 25a2152..b37b72e 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1321,35 +1321,36 @@ flymake-goto-diagnostic
    (flymake-show-diagnostic (if (button-type pos) (button-start pos) pos))))
 
 (defun flymake--diagnostics-buffer-entries ()
-  (with-current-buffer flymake--diagnostics-buffer-source
-    (cl-loop for diag in
-             (cl-sort (flymake-diagnostics) #'< :key #'flymake-diagnostic-beg)
-             for (line . col) =
-             (save-excursion
-               (goto-char (flymake--diag-beg diag))
-               (cons (line-number-at-pos)
-                     (- (point)
-                        (line-beginning-position))))
-             for type = (flymake--diag-type diag)
-             collect
-             (list (list :diagnostic diag
-                         :line line
-                         :severity (flymake--lookup-type-property
-                                    type
-                                    'severity (warning-numeric-level :error)))
-                   `[,(format "%s" line)
-                     ,(format "%s" col)
-                     ,(propertize (format "%s"
-                                          (flymake--lookup-type-property
-                                           type 'flymake-type-name type))
-                                  'face (flymake--lookup-type-property
-                                         type 'mode-line-face 'flymake-error))
-                     (,(format "%s" (flymake--diag-text diag))
-                      mouse-face highlight
-                      help-echo "mouse-2: visit this diagnostic"
-                      face nil
-                      action flymake-goto-diagnostic
-                      mouse-action flymake-goto-diagnostic)]))))
+  (when (bufferp flymake--diagnostics-buffer-source)
+    (with-current-buffer flymake--diagnostics-buffer-source
+      (cl-loop for diag in
+               (cl-sort (flymake-diagnostics) #'< :key #'flymake-diagnostic-beg)
+               for (line . col) =
+               (save-excursion
+                 (goto-char (flymake--diag-beg diag))
+                 (cons (line-number-at-pos)
+                       (- (point)
+                          (line-beginning-position))))
+               for type = (flymake--diag-type diag)
+               collect
+               (list (list :diagnostic diag
+                           :line line
+                           :severity (flymake--lookup-type-property
+                                      type
+                                      'severity (warning-numeric-level :error)))
+                     `[,(format "%s" line)
+                       ,(format "%s" col)
+                       ,(propertize (format "%s"
+                                            (flymake--lookup-type-property
+                                             type 'flymake-type-name type))
+                                    'face (flymake--lookup-type-property
+                                           type 'mode-line-face 'flymake-error))
+                       (,(format "%s" (flymake--diag-text diag))
+                        mouse-face highlight
+                        help-echo "mouse-2: visit this diagnostic"
+                        face nil
+                        action flymake-goto-diagnostic
+                        mouse-action flymake-goto-diagnostic)])))))
 
 (define-derived-mode flymake-diagnostics-buffer-mode tabulated-list-mode
   "Flymake diagnostics"

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Aidan Beggs]

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

That patch seems to do the trick for me. I don't make particularly
extensive use of the flymake diagnostics buffer, but for regular use it
seems to work as expected.

Thanks,
Aidan

On Sun, Apr 12, 2020 at 1:15 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Sun, 12 Apr 2020 17:58:16 +0100
> > Cc: Aidan Beggs <nadiasggeb001@gmail.com>, 40529@debbugs.gnu.org
> >
> > > I'm sure a simple solution for Flymake can be found.  E.g., what about
> > > skipping the entire body of flymake--diagnostics-buffer-entries if
> > > flymake--diagnostics-buffer-source is nil
> >
> > Maybe that works, yes. Feel free to try it and commit it
> > to Emacs 27, I have little time and I'm booted into a machine
> > with no Emacs.
>
> OK.
>
> Aidan, can you try the patch below and see if it solves the problem?
> The initial error in the recipe you posted is definitely gone after
> applying the patch, but please also try this after that and see that
> flymake-show-diagnostics-buffer also works after that, in real-life
> use.
>
> Thanks.
>
> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
> index 25a2152..b37b72e 100644
> --- a/lisp/progmodes/flymake.el
> +++ b/lisp/progmodes/flymake.el
> @@ -1321,35 +1321,36 @@ flymake-goto-diagnostic
>     (flymake-show-diagnostic (if (button-type pos) (button-start pos)
> pos))))
>
>  (defun flymake--diagnostics-buffer-entries ()
> -  (with-current-buffer flymake--diagnostics-buffer-source
> -    (cl-loop for diag in
> -             (cl-sort (flymake-diagnostics) #'< :key
> #'flymake-diagnostic-beg)
> -             for (line . col) =
> -             (save-excursion
> -               (goto-char (flymake--diag-beg diag))
> -               (cons (line-number-at-pos)
> -                     (- (point)
> -                        (line-beginning-position))))
> -             for type = (flymake--diag-type diag)
> -             collect
> -             (list (list :diagnostic diag
> -                         :line line
> -                         :severity (flymake--lookup-type-property
> -                                    type
> -                                    'severity (warning-numeric-level
> :error)))
> -                   `[,(format "%s" line)
> -                     ,(format "%s" col)
> -                     ,(propertize (format "%s"
> -                                          (flymake--lookup-type-property
> -                                           type 'flymake-type-name type))
> -                                  'face (flymake--lookup-type-property
> -                                         type 'mode-line-face
> 'flymake-error))
> -                     (,(format "%s" (flymake--diag-text diag))
> -                      mouse-face highlight
> -                      help-echo "mouse-2: visit this diagnostic"
> -                      face nil
> -                      action flymake-goto-diagnostic
> -                      mouse-action flymake-goto-diagnostic)]))))
> +  (when (bufferp flymake--diagnostics-buffer-source)
> +    (with-current-buffer flymake--diagnostics-buffer-source
> +      (cl-loop for diag in
> +               (cl-sort (flymake-diagnostics) #'< :key
> #'flymake-diagnostic-beg)
> +               for (line . col) =
> +               (save-excursion
> +                 (goto-char (flymake--diag-beg diag))
> +                 (cons (line-number-at-pos)
> +                       (- (point)
> +                          (line-beginning-position))))
> +               for type = (flymake--diag-type diag)
> +               collect
> +               (list (list :diagnostic diag
> +                           :line line
> +                           :severity (flymake--lookup-type-property
> +                                      type
> +                                      'severity (warning-numeric-level
> :error)))
> +                     `[,(format "%s" line)
> +                       ,(format "%s" col)
> +                       ,(propertize (format "%s"
> +                                            (flymake--lookup-type-property
> +                                             type 'flymake-type-name
> type))
> +                                    'face (flymake--lookup-type-property
> +                                           type 'mode-line-face
> 'flymake-error))
> +                       (,(format "%s" (flymake--diag-text diag))
> +                        mouse-face highlight
> +                        help-echo "mouse-2: visit this diagnostic"
> +                        face nil
> +                        action flymake-goto-diagnostic
> +                        mouse-action flymake-goto-diagnostic)])))))
>
>  (define-derived-mode flymake-diagnostics-buffer-mode tabulated-list-mode
>    "Flymake diagnostics"

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

bug#40529: 26.3; global-display-line-numbers-mode and flymake-show-diagnostics-buffer error

[Eli Zaretskii]

> From: Aidan Beggs <nadiasggeb001@gmail.com>
> Date: Sun, 12 Apr 2020 16:45:18 -0400
> Cc: João Távora <joaotavora@gmail.com>, 
> 	40529@debbugs.gnu.org
> 
> That patch seems to do the trick for me. I don't make particularly extensive use of the flymake diagnostics
> buffer, but for regular use it seems to work as expected.

Thanks, I've now installed the patch on the emacs-27 branch, and I'm
marking this bug done.  Feel free to reopen if there are more problems
related to this.