bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

[Dmitry Gutov]

...and the diagnostics buffer doesn't show anything either. Only the
mode-line indicator does.

Example: Rubocop reports the "E: unexpected token $end" errors with line
and column corresponding to the last position in the buffer.

flymake-diag-region translates that pair into a (EOB . (1+ EOB)) region.

And, apparently, flymake--highlight-line creates an evaporating overlay
with these buffer positions.

Maybe flymake-diag-region should check for (eob) and maybe backtrack a
little. Flycheck, in such situations, highlights the last symbol of the 
buffer.

++

In GNU Emacs 26.0.90 (build 5, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
  of 2017-11-07 built on zappa
Repository revision: ca2d94ba61dee678f85bfc7299d167e7219e6d8f
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
System Description:	Ubuntu 17.04

bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Maybe flymake-diag-region should check for (eob) and maybe backtrack a
> little. Flycheck, in such situations, highlights the last symbol of
> the buffer.

"Backtracking a little" sounds OK but highlighting the last symbol is a
little more contentious and harder to do (though I'm open to that
argument).

Anyway, this is a simple bug because my original idea was to make this
case behave like the case where the last line is referenced but without
a column indication. Inside flymake-diag-region, this should funnel into
fallback-eol but in this particular case it wasn't doind that because I
forgot that (goto-char one-trillion) doesn't error.

Fixed in 535688a4181ae4052db354ce2b877507f11c9e66.

Thanks,
João

bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

[Dmitry Gutov]

On 11/9/17 11:18 PM, João Távora wrote:

> "Backtracking a little" sounds OK but highlighting the last symbol is a
> little more contentious and harder to do (though I'm open to that
> argument).

That's okay.

> Anyway, this is a simple bug because my original idea was to make this
> case behave like the case where the last line is referenced but without
> a column indication. Inside flymake-diag-region, this should funnel into
> fallback-eol but in this particular case it wasn't doind that because I
> forgot that (goto-char one-trillion) doesn't error.
> 
> Fixed in 535688a4181ae4052db354ce2b877507f11c9e66.

The idea sounds fine, but it doesn't work when the last line of the 
buffer is empty (e.g. when the file ends with a newline, like with 
require-final-newline set to t): highlighting the last line still ends 
up creating a zero-size overlay.

So I think the important point is to skip the trailing whitespace first. 
Maybe this calls for a test case or two.

bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> So I think the important point is to skip the trailing whitespace
> first. Maybe this calls for a test case or two.

Indeed, the tests to flymake-diag-region should be 6-fold though. Two
tests, with and without column indication, where eob is being pointed
to, for each of these buffer fixtures

   w/o trailing newline -> highlight last line of visible chars
   with trailing newline -> highlight last line of visible chars
   with two trailing newlines -> highlight wide last line of whitespace

...but I'm too lazy at this time of night to do the tests, though I
do have the patch I think should fix them :-)

Thanks,
João

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index b4ab7f223f..241ea00d64 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -318,7 +318,11 @@ flymake-diag-region
             (goto-char (point-min))
             (forward-line (1- line))
             (cl-flet ((fallback-bol
-                       () (progn (back-to-indentation) (point)))
+                       ()
+                       (back-to-indentation)
+                       (if (eobp)
+                           (line-beginning-position 0)
+                         (point)))
                       (fallback-eol
                        (beg)
                        (progn
@@ -335,11 +339,11 @@ flymake-diag-region
                                        (not (= sexp-end beg))
                                        sexp-end)
                                   (and (< (goto-char (1+ beg)) (point-max))
-                                       (point))))
-                         (safe-end (or end
-                                       (fallback-eol beg))))
-                    (cons (if end beg (fallback-bol))
-                          safe-end))
+                                       (point)))))
+                    (if end
+                        (cons beg end)
+                      (cons (setq beg (fallback-bol))
+                            (fallback-eol beg))))
                 (let* ((beg (fallback-bol))
                        (end (fallback-eol beg)))
                   (cons beg end)))))))

bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB

[Dmitry Gutov]

On 11/10/17 7:32 AM, João Távora wrote:

> ...but I'm too lazy at this time of night to do the tests, though I
> do have the patch I think should fix them :-)

This one works, thanks!

 >     with two trailing newlines -> highlight wide last line of whitespace

This is a bit questionable, but good enough, considering one shouldn't 
have multilple trailing newlines anyway.