all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB
@ 2017-11-07 23:26 Dmitry Gutov
  2017-11-09 21:18 ` João Távora
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Gutov @ 2017-11-07 23:26 UTC (permalink / raw)
  To: 29201

...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





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB
  2017-11-07 23:26 bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB Dmitry Gutov
@ 2017-11-09 21:18 ` João Távora
  2017-11-10  0:02   ` Dmitry Gutov
  0 siblings, 1 reply; 5+ messages in thread
From: João Távora @ 2017-11-09 21:18 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 29201

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





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB
  2017-11-09 21:18 ` João Távora
@ 2017-11-10  0:02   ` Dmitry Gutov
  2017-11-10  5:32     ` João Távora
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Gutov @ 2017-11-10  0:02 UTC (permalink / raw)
  To: João Távora; +Cc: 29201

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.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB
  2017-11-10  0:02   ` Dmitry Gutov
@ 2017-11-10  5:32     ` João Távora
  2017-11-10 11:19       ` Dmitry Gutov
  0 siblings, 1 reply; 5+ messages in thread
From: João Távora @ 2017-11-10  5:32 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 29201

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






^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB
  2017-11-10  5:32     ` João Távora
@ 2017-11-10 11:19       ` Dmitry Gutov
  0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Gutov @ 2017-11-10 11:19 UTC (permalink / raw)
  To: João Távora; +Cc: 29201

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.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-10 11:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-07 23:26 bug#29201: 26.0.90; Flymake skips indicator when a backend reports a diagnostic at EOB Dmitry Gutov
2017-11-09 21:18 ` João Távora
2017-11-10  0:02   ` Dmitry Gutov
2017-11-10  5:32     ` João Távora
2017-11-10 11:19       ` Dmitry Gutov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.