bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

X-Debbugs-Cc: joaotavora@gmail.com

The faces 'flymake-error-echo' and 'flymake-warning-echo' inherit from 
'compilation-error' and 'compilation-warning', respectively. However, 
'flymake-note-echo' (and 'flymake-note-echo-at-eol') inherit from 
'flymake-note'.

This results in some odd visuals in "emacs -Q" (and any theme that uses 
similar styling): Flymake errors and warnings in the mode-line (or the 
minibuffer/end-of-line) are red or orange text. However, Flymake notes 
are the default text color with a green wavey underline. Especially when 
they're side-by-side in the mode-line, the current visuals are 
surprising (not to mention hard to see in the default theme).

Personally, I think it'd be better for the "echo" faces for Flymake 
notes to just inherit from 'compilation-info'. They have almost 
identical uses as far as I can tell, and then things would look more 
consistent/readable. (In fact, we could even rename flymake-note to 
flymake-info, but maybe that ship has sailed.)

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

Hello Jim,

On Sun, Sep 17, 2023 at 5:42 AM Jim Porter <jporterbugs@gmail.com> wrote:


> Personally, I think it'd be better for the "echo" faces for Flymake
> notes to just inherit from 'compilation-info'. They have almost
> identical uses as far as I can tell, and then things would look more

To better understand what you mean, can you post two screenshots -- with
and without your proposed change -- of what the default theme would look
like in a simple file with some Flymake overlays?

Also, a patch showing exactly the change you propose would be useful.  And
keep in mind one can define a face to inherit from two faces (AFAIK).

> consistent/readable. (In fact, we could even rename flymake-note to
> flymake-info, but maybe that ship has sailed.)

We could rename and add a backward compatibility alias.  But lets first
establish if that makes sense: "note" comes from the GCC nomenclature
for informational messages.  Also I'm not 100% "note" wasn't already
used in Flymake before i rewrote it.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

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

On 9/17/2023 2:22 PM, João Távora wrote:
> On Sun, Sep 17, 2023 at 5:42 AM Jim Porter <jporterbugs@gmail.com> wrote:
> 
>> Personally, I think it'd be better for the "echo" faces for Flymake
>> notes to just inherit from 'compilation-info'. They have almost
>> identical uses as far as I can tell, and then things would look more
> 
> To better understand what you mean, can you post two screenshots -- with
> and without your proposed change -- of what the default theme would look
> like in a simple file with some Flymake overlays?

Sure, here are screenshots of Flymake under 29, 30, and 30 plus my 
patch. In particular, pay close attention to the mode-line colors. It 
took me a few seconds of close inspection to see the green underline for 
the note counter in the mode-line.

> Also, a patch showing exactly the change you propose would be useful.  And
> keep in mind one can define a face to inherit from two faces (AFAIK).

And here's also a patch. (Not for the renaming of note->info, but that's 
straightforward enough to imagine, I hope.)

[-- Attachment #2: emacs-29.png --]
[-- Type: image/png, Size: 17034 bytes --]

[-- Attachment #3: emacs-30.png --]
[-- Type: image/png, Size: 20189 bytes --]

[-- Attachment #4: emacs-30-patched.png --]
[-- Type: image/png, Size: 20320 bytes --]

[-- Attachment #5: 0001-Use-compilation-info-as-the-basis-for-Flymake-note-f.patch --]
[-- Type: text/plain, Size: 1276 bytes --]

From 26c26763bed97f97fb795012c06cb91dfd28de5c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 17 Sep 2023 14:50:22 -0700
Subject: [PATCH] Use 'compilation-info' as the basis for Flymake note faces

* lisp/progmodes/flymake.el (flymake-note-echo)
(flymake-note-echo-at-eol): Inherit from 'compilation-info'.
---
 lisp/progmodes/flymake.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index b044a661911..66aff1d1fe9 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -442,7 +442,7 @@ flymake-warning-echo
   :package-version '(Flymake . "1.3.4"))
 
 (defface flymake-note-echo
-  '((t :inherit flymake-note))
+  '((t :inherit compilation-info))
   "Face used for showing summarized descriptions of notes."
   :package-version '(Flymake . "1.3.4"))
 
@@ -463,7 +463,7 @@ flymake-warning-echo-at-eol
   :package-version '(Flymake . "1.3.5"))
 
 (defface flymake-note-echo-at-eol
-  '((t :inherit (flymake-end-of-line-diagnostics-face flymake-note)))
+  '((t :inherit (flymake-end-of-line-diagnostics-face compilation-info)))
   "Face like `flymake-note-echo', but for end-of-line overlays."
   :package-version '(Flymake . "1.3.5"))
 
-- 
2.25.1

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Sun, Sep 17, 2023 at 10:54 PM Jim Porter <jporterbugs@gmail.com> wrote:
>
> On 9/17/2023 2:22 PM, João Távora wrote:
> > On Sun, Sep 17, 2023 at 5:42 AM Jim Porter <jporterbugs@gmail.com> wrote:
> >
> >> Personally, I think it'd be better for the "echo" faces for Flymake
> >> notes to just inherit from 'compilation-info'. They have almost
> >> identical uses as far as I can tell, and then things would look more
> >
> > To better understand what you mean, can you post two screenshots -- with
> > and without your proposed change -- of what the default theme would look
> > like in a simple file with some Flymake overlays?
>
> Sure, here are screenshots of Flymake under 29, 30, and 30 plus my
> patch. In particular, pay close attention to the mode-line colors. It
> took me a few seconds of close inspection to see the green underline for
> the note counter in the mode-line.

OK thanks for showing this.  I'll think about this better tomorrow.

In the meantime, I found two other bugs trying this recipe.  First
let's write down the recipe:

emacs -Q
M-x flymake-mode RET
M-: (setq flymake-show-diagnostics-at-end-of-line t) RET
M-x erase-buffer RET
h e l l o

The first bug I find is that the exclamation mark on the fringe is
green, but it should be yellow, since, presumably a warning is
more important than a note.

The second bug has to do with the placement of the cursor when
two "eol overlays" are present.  The cursor shouldn't be placed
in the middle of them, since they represent unreachable areas of
the buffer.

These two bugs are IMO a bit more serious, but I also see your
point about the faces.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

On 9/17/2023 3:15 PM, João Távora wrote:
> The first bug I find is that the exclamation mark on the fringe is
> green, but it should be yellow, since, presumably a warning is
> more important than a note.
> 
> The second bug has to do with the placement of the cursor when
> two "eol overlays" are present.  The cursor shouldn't be placed
> in the middle of them, since they represent unreachable areas of
> the buffer.
> 
> These two bugs are IMO a bit more serious, but I also see your
> point about the faces.

Yeah, those are probably a bigger deal. I'd noticed them, but didn't 
think very hard about whether they were right or not since at the time I 
was mostly in "editing bits of my theme" mode.

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> Cc: 66041@debbugs.gnu.org
> From: João Távora <joaotavora@gmail.com>
> Date: Sun, 17 Sep 2023 23:15:31 +0100
> 
> emacs -Q
> M-x flymake-mode RET
> M-: (setq flymake-show-diagnostics-at-end-of-line t) RET
> M-x erase-buffer RET
> h e l l o
> 
> The first bug I find is that the exclamation mark on the fringe is
> green, but it should be yellow, since, presumably a warning is
> more important than a note.
> 
> The second bug has to do with the placement of the cursor when
> two "eol overlays" are present.  The cursor shouldn't be placed
> in the middle of them, since they represent unreachable areas of
> the buffer.

When I try the above, the cursor is at the end of "hello", and the
overlay with the warning is displayed after it.  So I don't think I
understand what you mean by "in the middle of them".

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

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

On Mon, Sep 18, 2023, 11:44 Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 66041@debbugs.gnu.org
> > From: João Távora <joaotavora@gmail.com>
> > Date: Sun, 17 Sep 2023 23:15:31 +0100
> >
> > emacs -Q
> > M-x flymake-mode RET
> > M-: (setq flymake-show-diagnostics-at-end-of-line t) RET
> > M-x erase-buffer RET
> > h e l l o
> >
> > The first bug I find is that the exclamation mark on the fringe is
> > green, but it should be yellow, since, presumably a warning is
> > more important than a note.
> >
> > The second bug has to do with the placement of the cursor when
> > two "eol overlays" are present.  The cursor shouldn't be placed
> > in the middle of them, since they represent unreachable areas of
> > the buffer.
>
> When I try the above, the cursor is at the end of "hello", and the
> overlay with the warning is displayed after it.  So I don't think I
> understand what you mean by "in the middle of them".
>

Sorry. The recipe is missing a single SPC character after the hello, so
that the style warning is triggered.

João

>

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

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 18 Sep 2023 11:46:55 +0100
> Cc: Jim Porter <jporterbugs@gmail.com>, 66041@debbugs.gnu.org
> 
> On Mon, Sep 18, 2023, 11:44 Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  > Cc: 66041@debbugs.gnu.org
>  > From: João Távora <joaotavora@gmail.com>
>  > Date: Sun, 17 Sep 2023 23:15:31 +0100
>  > 
>  > emacs -Q
>  > M-x flymake-mode RET
>  > M-: (setq flymake-show-diagnostics-at-end-of-line t) RET
>  > M-x erase-buffer RET
>  > h e l l o
>  > 
>  > The first bug I find is that the exclamation mark on the fringe is
>  > green, but it should be yellow, since, presumably a warning is
>  > more important than a note.
>  > 
>  > The second bug has to do with the placement of the cursor when
>  > two "eol overlays" are present.  The cursor shouldn't be placed
>  > in the middle of them, since they represent unreachable areas of
>  > the buffer.
> 
>  When I try the above, the cursor is at the end of "hello", and the
>  overlay with the warning is displayed after it.  So I don't think I
>  understand what you mean by "in the middle of them".
> 
> Sorry. The recipe is missing a single SPC character after the hello, so that the style warning is
> triggered.

You have there two overlays, each one with a before-string, and each
string has its first character propertized with (cursor t).  So Emacs
picks up one of the two overlay strings to place the cursor, and it
just happens to be not the one you wanted.

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 18, 2023 at 12:42 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Mon, 18 Sep 2023 11:46:55 +0100
> > Cc: Jim Porter <jporterbugs@gmail.com>, 66041@debbugs.gnu.org
> >
> > On Mon, Sep 18, 2023, 11:44 Eli Zaretskii <eliz@gnu.org> wrote:
> >
> >  > Cc: 66041@debbugs.gnu.org
> >  > From: João Távora <joaotavora@gmail.com>
> >  > Date: Sun, 17 Sep 2023 23:15:31 +0100
> >  >
> >  > emacs -Q
> >  > M-x flymake-mode RET
> >  > M-: (setq flymake-show-diagnostics-at-end-of-line t) RET
> >  > M-x erase-buffer RET
> >  > h e l l o
> >  >
> >  > The first bug I find is that the exclamation mark on the fringe is
> >  > green, but it should be yellow, since, presumably a warning is
> >  > more important than a note.
> >  >
> >  > The second bug has to do with the placement of the cursor when
> >  > two "eol overlays" are present.  The cursor shouldn't be placed
> >  > in the middle of them, since they represent unreachable areas of
> >  > the buffer.
> >
> >  When I try the above, the cursor is at the end of "hello", and the
> >  overlay with the warning is displayed after it.  So I don't think I
> >  understand what you mean by "in the middle of them".
> >
> > Sorry. The recipe is missing a single SPC character after the hello, so that the style warning is
> > triggered.
>
> You have there two overlays, each one with a before-string, and each
> string has its first character propertized with (cursor t).  So Emacs
> picks up one of the two overlay strings to place the cursor, and it
> just happens to be not the one you wanted.

Yes, something like that.  Skimming the code, I think I meant for only
one overlay, not two, to be the end-of-line overlay containing the two
strings.  But this was tricky to implement and I probably missed an
edge case.  There is a FIXME there, have to investigate.

Anyway, since I have your interest, any suggestions on how you would
implement this? Knowing that this feature is upposed to display
multiple pieces of relatively short cursor-unreachable text visually
after the end -of-line (the text being the diagnostic text, naturally).

Currently I'm placing them exactly between (line-end-position) and the
character after that.  There is a link between this eol overlay and
the origin diagnostic.  If you delete the latter, the former should
be recalculated asap, i.e. it should ideally not wait another 1s or two
before Flymake re-contacts the backend for up-to-date info.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 18 Sep 2023 13:52:06 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> On Mon, Sep 18, 2023 at 12:42 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > You have there two overlays, each one with a before-string, and each
> > string has its first character propertized with (cursor t).  So Emacs
> > picks up one of the two overlay strings to place the cursor, and it
> > just happens to be not the one you wanted.
> 
> Yes, something like that.  Skimming the code, I think I meant for only
> one overlay, not two, to be the end-of-line overlay containing the two
> strings.  But this was tricky to implement and I probably missed an
> edge case.  There is a FIXME there, have to investigate.
> 
> Anyway, since I have your interest, any suggestions on how you would
> implement this? Knowing that this feature is upposed to display
> multiple pieces of relatively short cursor-unreachable text visually
> after the end -of-line (the text being the diagnostic text, naturally).

I guess you want the cursor on the first character of the
overlay-string that is displayed first (leftmost)?  Are you asking how
to implement this when there are more than one overlay at EOB?

> Currently I'm placing them exactly between (line-end-position) and the
> character after that.  There is a link between this eol overlay and
> the origin diagnostic.  If you delete the latter, the former should
> be recalculated asap, i.e. it should ideally not wait another 1s or two
> before Flymake re-contacts the backend for up-to-date info.

This seems to hint that you are talking about something different, so
maybe I misunderstand what you mean by "implement this" above?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 18, 2023 at 3:32 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Mon, 18 Sep 2023 13:52:06 +0100
> > Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> >
> > On Mon, Sep 18, 2023 at 12:42 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > You have there two overlays, each one with a before-string, and each
> > > string has its first character propertized with (cursor t).  So Emacs
> > > picks up one of the two overlay strings to place the cursor, and it
> > > just happens to be not the one you wanted.
> >
> > Yes, something like that.  Skimming the code, I think I meant for only
> > one overlay, not two, to be the end-of-line overlay containing the two
> > strings.  But this was tricky to implement and I probably missed an
> > edge case.  There is a FIXME there, have to investigate.
> >
> > Anyway, since I have your interest, any suggestions on how you would
> > implement this? Knowing that this feature is upposed to display
> > multiple pieces of relatively short cursor-unreachable text visually
> > after the end -of-line (the text being the diagnostic text, naturally).
>
> I guess you want the cursor on the first character of the
> overlay-string that is displayed first (leftmost)?  Are you asking how
> to implement this when there are more than one overlay at EOB?

Yes, a direct answer to that question would be nice to know, too.

> > Currently I'm placing them exactly between (line-end-position) and the
> > character after that.  There is a link between this eol overlay and
> > the origin diagnostic.  If you delete the latter, the former should
> > be recalculated asap, i.e. it should ideally not wait another 1s or two
> > before Flymake re-contacts the backend for up-to-date info.
>
> This seems to hint that you are talking about something different, so
> maybe I misunderstand what you mean by "implement this" above?

By my question, I just meant that I'm interested in hearing
what approach you would take to solve this problem.  Let me
write a bit more.

The idea is to have multiple bits of diagnostic text displayed
after the end-of-line each related to a source overlay contained
within that line.  The idea of the feature is that source overlay
and eol text are linked.  If the source overlay is deleted, the
end-of-line should disappear.

It's similar to what happens with a simple 'after-string' overlay
property, but with the 'after-string' skipping over all characters
until the end of line.

Currently, I'm doing this with a single "end-of-line" overlay placed
as I explained properly (modulo bugs like this one, of course, since
I intended a single overlay but unexpectedly got two).  It's this
second end-of-line overlay which has an 'after-string' property. I use
after change functions to control the value of this property, so that
if a source overlays gets destroyed, the corresponding text in value
is deleted (and eventually also

But I could probably use multiple end-of-line overlays, with a
one-to-one correspondence to the source overlays.  I've tried that too
but abandoned it for difficulty in placing the cursor (similar problem
to this one).

And I think I tried an overlay modification hook instead of buffer after
change functions and abandoned it too for some reason.  Maybe the
modification hook isn't called at all when the overlay is completely
deleted?

Or, conceivably, it could be done without end-of-line overlays at all,
if somehow a 'eol-string' property similar to 'after-string' was
implemented in the C code.  But I would prefer a Lisp solution, of
course.

So, in summary, there is a fair bit of design space for this feature
(which exists in VS code, btw) and I wanted to know your thoughts
on these possibilities and my decisions.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 18 Sep 2023 16:31:45 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> On Mon, Sep 18, 2023 at 3:32 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > I guess you want the cursor on the first character of the
> > overlay-string that is displayed first (leftmost)?  Are you asking how
> > to implement this when there are more than one overlay at EOB?
> 
> Yes, a direct answer to that question would be nice to know, too.

Give your overlays different priorities, and use (cursor t) only in
the overlay of the highest priority.  If they all have before-string
(which is what I saw in the example), then priority will determine
which overlay is processed first (and thus displayed first).

> The idea is to have multiple bits of diagnostic text displayed
> after the end-of-line each related to a source overlay contained
> within that line.  The idea of the feature is that source overlay
> and eol text are linked.  If the source overlay is deleted, the
> end-of-line should disappear.
> 
> It's similar to what happens with a simple 'after-string' overlay
> property, but with the 'after-string' skipping over all characters
> until the end of line.
> 
> Currently, I'm doing this with a single "end-of-line" overlay placed
> as I explained properly (modulo bugs like this one, of course, since
> I intended a single overlay but unexpectedly got two).  It's this
> second end-of-line overlay which has an 'after-string' property. I use
> after change functions to control the value of this property, so that
> if a source overlays gets destroyed, the corresponding text in value
> is deleted (and eventually also
> 
> But I could probably use multiple end-of-line overlays, with a
> one-to-one correspondence to the source overlays.  I've tried that too
> but abandoned it for difficulty in placing the cursor (similar problem
> to this one).
> 
> And I think I tried an overlay modification hook instead of buffer after
> change functions and abandoned it too for some reason.  Maybe the
> modification hook isn't called at all when the overlay is completely
> deleted?
> 
> Or, conceivably, it could be done without end-of-line overlays at all,
> if somehow a 'eol-string' property similar to 'after-string' was
> implemented in the C code.  But I would prefer a Lisp solution, of
> course.
> 
> So, in summary, there is a fair bit of design space for this feature
> (which exists in VS code, btw) and I wanted to know your thoughts
> on these possibilities and my decisions.

Is placing the cursor the only problem you need to solve?

And do I understand correctly that you normally collect all the
diagnostics into a single sting, and display it as a single overlay
string?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 18, 2023 at 6:29 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Mon, 18 Sep 2023 16:31:45 +0100
> > Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> >
> > On Mon, Sep 18, 2023 at 3:32 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > I guess you want the cursor on the first character of the
> > > overlay-string that is displayed first (leftmost)?  Are you asking how
> > > to implement this when there are more than one overlay at EOB?
> >
> > Yes, a direct answer to that question would be nice to know, too.
>
> Give your overlays different priorities, and use (cursor t) only in
> the overlay of the highest priority.  If they all have before-string
> (which is what I saw in the example), then priority will determine
> which overlay is processed first (and thus displayed first).
>
> > The idea is to have multiple bits of diagnostic text displayed
> > after the end-of-line each related to a source overlay contained
> > within that line.  The idea of the feature is that source overlay
> > and eol text are linked.  If the source overlay is deleted, the
> > end-of-line should disappear.
> >
> > It's similar to what happens with a simple 'after-string' overlay
> > property, but with the 'after-string' skipping over all characters
> > until the end of line.
> >
> > Currently, I'm doing this with a single "end-of-line" overlay placed
> > as I explained properly (modulo bugs like this one, of course, since
> > I intended a single overlay but unexpectedly got two).  It's this
> > second end-of-line overlay which has an 'after-string' property. I use
> > after change functions to control the value of this property, so that
> > if a source overlays gets destroyed, the corresponding text in value
> > is deleted (and eventually also
> >
> > But I could probably use multiple end-of-line overlays, with a
> > one-to-one correspondence to the source overlays.  I've tried that too
> > but abandoned it for difficulty in placing the cursor (similar problem
> > to this one).
> >
> > And I think I tried an overlay modification hook instead of buffer after
> > change functions and abandoned it too for some reason.  Maybe the
> > modification hook isn't called at all when the overlay is completely
> > deleted?
> >
> > Or, conceivably, it could be done without end-of-line overlays at all,
> > if somehow a 'eol-string' property similar to 'after-string' was
> > implemented in the C code.  But I would prefer a Lisp solution, of
> > course.
> >
> > So, in summary, there is a fair bit of design space for this feature
> > (which exists in VS code, btw) and I wanted to know your thoughts
> > on these possibilities and my decisions.
>
> Is placing the cursor the only problem you need to solve?

No.  It's one of them.  The other main problem is that the display-only
string carried by the overlay needs to react quickly to deletions of
the "source overlays" that span ranges of characters in the line to
the left (or to the right, if writing in the opposite direction, but
I hope you get what I mean).

> And do I understand correctly that you normally collect all the
> diagnostics into a single sting, and display it as a single overlay
> string?

For purposes of the "end of line" overlay, yes.  Max one per line.
That was the intent at least.  But it seemingly failed here, so we got
two for that first line.

However there are more overlays.  These "end of line" overlays only
appear once you activate flymake-show-diagnostics-at-end-of-line.
But Flymake uses other overlays to underline and propertize the
actual text that originated the diagnostic "inside the line".  These
I call the "source overlays": they are the source of truth for the
diagnostic information and its their deletion that should immediately
trigger an update of the "end of line" overlay text.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

On 9/18/2023 5:52 AM, João Távora wrote:
> Yes, something like that.  Skimming the code, I think I meant for only
> one overlay, not two, to be the end-of-line overlay containing the two
> strings.  But this was tricky to implement and I probably missed an
> edge case.  There is a FIXME there, have to investigate.
> 
> Anyway, since I have your interest, any suggestions on how you would
> implement this? Knowing that this feature is upposed to display
> multiple pieces of relatively short cursor-unreachable text visually
> after the end -of-line (the text being the diagnostic text, naturally).

For what it's worth, I think it would be nice to have a single overlay, 
showing only the highest-severity message, and if there are more, add a 
"(+ N more)" suffix. Then, assuming it's possible to implement, you 
could click/hover over it to get a popup of some sort with all the 
messages. That way you can't overload the UI with many messages (which 
in practice might all be from the same underlying problem in your code).

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 18, 2023 at 6:44 PM Jim Porter <jporterbugs@gmail.com> wrote:
>
> On 9/18/2023 5:52 AM, João Távora wrote:
> > Yes, something like that.  Skimming the code, I think I meant for only
> > one overlay, not two, to be the end-of-line overlay containing the two
> > strings.  But this was tricky to implement and I probably missed an
> > edge case.  There is a FIXME there, have to investigate.
> >
> > Anyway, since I have your interest, any suggestions on how you would
> > implement this? Knowing that this feature is upposed to display
> > multiple pieces of relatively short cursor-unreachable text visually
> > after the end -of-line (the text being the diagnostic text, naturally).
>
> For what it's worth, I think it would be nice to have a single overlay,
> showing only the highest-severity message, and if there are more, add a
> "(+ N more)" suffix. Then, assuming it's possible to implement, you
> could click/hover over it to get a popup of some sort with all the
> messages. That way you can't overload the UI with many messages (which
> in practice might all be from the same underlying problem in your code).

That's possible to implement.

But the single/multiple overlay decision is just an implementation detail
Currently Flymake uses one "end of line" overlay only, but here the logic
o control that uniqueness failed, so an extra unintended overlay was
created, and that created problems with cursor placement.

But your suggestion is noted, though.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

On 9/18/2023 11:49 AM, João Távora wrote:
> That's possible to implement.
> 
> But the single/multiple overlay decision is just an implementation detail
> Currently Flymake uses one "end of line" overlay only, but here the logic
> o control that uniqueness failed, so an extra unintended overlay was
> created, and that created problems with cursor placement.
> 
> But your suggestion is noted, though.

Ah ha, that makes sense. I thought the multiple overlays was intentional.

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 18, 2023 at 7:49 PM João Távora <joaotavora@gmail.com> wrote:

> That's possible to implement.
>
> But the single/multiple overlay decision is just an implementation detail
> Currently Flymake uses one "end of line" overlay only, but here the logic
> o control that uniqueness failed, so an extra unintended overlay was
> created, and that created problems with cursor placement.
>
> But your suggestion is noted, though.

Over the last 2 days, I've been working on this bug and pushed your patch.

I also significantly reworked and robustified the end-of-line overlay logic,
fixing the two bugs I had identified.

Then I also implemented your "and N more" suffix suggestion.  Just set
flymake-show-diagnostic-at-end-of-line to 'short'.

I'm closing this bug, but let me know if you are satisfied with the changes
or notice more points of possible improvement.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

On 9/21/2023 2:40 PM, João Távora wrote:
> I'm closing this bug, but let me know if you are satisfied with the changes
> or notice more points of possible improvement.

Looks good to me. One question though: I don't see a change for the 
issue with the color of the exclamation point in the fringe (testing on 
commit 12fe4741b6f). Was that something you still wanted to fix? (See 
below.)

On 9/17/2023 3:15 PM, João Távora wrote:
> The first bug I find is that the exclamation mark on the fringe is
> green, but it should be yellow, since, presumably a warning is
> more important than a note

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Sun, Sep 24, 2023 at 4:38 AM Jim Porter <jporterbugs@gmail.com> wrote:
>
> On 9/21/2023 2:40 PM, João Távora wrote:
> > I'm closing this bug, but let me know if you are satisfied with the changes
> > or notice more points of possible improvement.
>
> Looks good to me. One question though: I don't see a change for the
> issue with the color of the exclamation point in the fringe (testing on
> commit 12fe4741b6f). Was that something you still wanted to fix? (See
> below.)

Yes, I thought I had seen it sort itself out, but it didn't, I just
confirmed.

Shouldn't be very hard, I'll look at this later this evening.

João



-- 
João Távora

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Sun, Sep 24, 2023 at 9:18 AM João Távora <joaotavora@gmail.com> wrote:

> Yes, I thought I had seen it sort itself out, but it didn't, I just
> confirmed.
>
> Shouldn't be very hard, I'll look at this later this evening.

As usual, this turned out not to be true.  It seems that even if
though the overlay dedicated to the  "warning" diagnostic has
higher priority than the overlay dedicated to the "note/info"
diagnostic the latter's 'before-string' property, which
is:

  #("!" 0 1 (display (left-fringe exclamation-mark compilation-info)))

still trumps the former's, which is:

  #("!" 0 1 (display (left-fringe exclamation-mark compilation-warning)))

The priorities being used here are of the (PRIMARY . SECONDARY) form.
Maybe that has something to do with it, but I tried just "normal"
fixnum priorities and that didn't fix it.

Pinging Eli again.  Eli, here's the recipe again:

  emacs -Q -f flymake-mode
  M-x erase-buffer RET y
  h e l l o SPC

On the left fringe, we expect to see an orange exclamation mark
(compilation-warning face). Instead, we see a green one
(compilation-info face).

Using M-x describe-text-properties, one can see two overlays, one
from 1 to 6, another from 6 to 7, with priorities (nil . 42) and
(nil . 41), respectively.

The 'before-string' chosen for display on the fringe seems to have
more to do with the overlay that appears later in the line, rather
than with priorities.

Another thing worth mentioning is that this bug seems to happen in
both pre- and post- noverlay code by Stefan Monnier, since I can reproduce
back to Emacs 28.

Also worth noting that this is a bug of relatively minor importance
IMO.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 25 Sep 2023 09:59:22 +0100
> Cc: 66041@debbugs.gnu.org
> 
> As usual, this turned out not to be true.  It seems that even if
> though the overlay dedicated to the  "warning" diagnostic has
> higher priority than the overlay dedicated to the "note/info"
> diagnostic the latter's 'before-string' property, which
> is:
> 
>   #("!" 0 1 (display (left-fringe exclamation-mark compilation-info)))
> 
> still trumps the former's, which is:
> 
>   #("!" 0 1 (display (left-fringe exclamation-mark compilation-warning)))
> 
> The priorities being used here are of the (PRIMARY . SECONDARY) form.
> Maybe that has something to do with it, but I tried just "normal"
> fixnum priorities and that didn't fix it.
> 
> Pinging Eli again.  Eli, here's the recipe again:
> 
>   emacs -Q -f flymake-mode
>   M-x erase-buffer RET y
>   h e l l o SPC
> 
> On the left fringe, we expect to see an orange exclamation mark
> (compilation-warning face). Instead, we see a green one
> (compilation-info face).
> 
> Using M-x describe-text-properties, one can see two overlays, one
> from 1 to 6, another from 6 to 7, with priorities (nil . 42) and
> (nil . 41), respectively.
> 
> The 'before-string' chosen for display on the fringe seems to have
> more to do with the overlay that appears later in the line, rather
> than with priorities.

Add some character after the SPC, even a newline, and the display will
be as you expect.

Do we care about the corner case of the last character before EOB?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 25, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:

> Add some character after the SPC, even a newline, and the display will
> be as you expect.

I think that's only because this newline will, by default, delete the SPC
character and thus resolve the diagnostic from checkdoc, deleting the
overlay.

But there are very many ways in which a lower priority diagnostic
may appear later in the same line as a higher priority one, and
I think this will trigger the problem.

> Do we care about the corner case of the last character before EOB?

If it were that, I'd say "no, we don't", but it doesn't seem
like it is.

I think we should care _somewhat_ about this recipe, which represents
the aforementioned use case, a case I don't think is particularly
contrived:

emacs -Q -f flymake-mode
M-x erase-buffer RET y
C-o
h e l l o SPC

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 25 Sep 2023 12:46:35 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> On Mon, Sep 25, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Add some character after the SPC, even a newline, and the display will
> > be as you expect.
> 
> I think that's only because this newline will, by default, delete the SPC
> character and thus resolve the diagnostic from checkdoc, deleting the
> overlay.

No, because if you insert any other character after the space, the
display will also be as you expect.

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 25, 2023 at 1:09 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Mon, 25 Sep 2023 12:46:35 +0100
> > Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> >
> > On Mon, Sep 25, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > Add some character after the SPC, even a newline, and the display will
> > > be as you expect.
> >
> > I think that's only because this newline will, by default, delete the SPC
> > character and thus resolve the diagnostic from checkdoc, deleting the
> > overlay.
>
> No, because if you insert any other character after the space, the
> display will also be as you expect.

Obviously, because any other character which is not space
will _also_ fix the trailing whitespace diagnostic issued by the
checkdoc backend. By definition :-)

I can give a more complicated recipe using the Eglot and LSP
if you want, but it will require you install 'clangd'.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 25 Sep 2023 13:12:50 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> On Mon, Sep 25, 2023 at 1:09 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: João Távora <joaotavora@gmail.com>
> > > Date: Mon, 25 Sep 2023 12:46:35 +0100
> > > Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> > >
> > > On Mon, Sep 25, 2023 at 11:32 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > >
> > > > Add some character after the SPC, even a newline, and the display will
> > > > be as you expect.
> > >
> > > I think that's only because this newline will, by default, delete the SPC
> > > character and thus resolve the diagnostic from checkdoc, deleting the
> > > overlay.
> >
> > No, because if you insert any other character after the space, the
> > display will also be as you expect.
> 
> Obviously, because any other character which is not space
> will _also_ fix the trailing whitespace diagnostic issued by the
> checkdoc backend. By definition :-)

OK, let's back up a notch, because I think I've misunderstood you.

What you have here are two separate overlays, each with a
before-string.  The overlays cover two completely disjoint portions of
buffer text: 1 to 6 for the first overlay, 6 to 7 for the second.

Now, priorities are only relevant for overlays that cover overlapping
portions of text.  In your case, there's no overlap, so both overlays
will be displayed.  Try this variant:

  (let (ov)
    (setq ov (make-overlay 1 6))
    (overlay-put ov 'before-string "B1")
    (overlay-put ov 'priority 42)
    (setq ov (make-overlay 6 7))
    (overlay-put ov 'before-string "B2")
    (overlay-put ov 'priority 41))

You will see that "B1" is displayed before "hello" and "B2" is
displayed before the space that follows "hello", like this:

  B1helloB2

Both overlays show on display regardless of their priorities.  Right?

That is exactly what happens in your case: both overlays are
displayed.  Except that in your case the before-strings have a
'display' property on them, whose value draws a bitmap on the fringe,
instead of showing the string itself in the text-area.  What Emacs
does is again display both overlays, both of which draw a bitmap on
the same fringe, so what you see is the bitmap which is drawn last.
Because when some 'display' property draws on the fringe, we draw it
unconditionally, regardless of priorities etc.  So the last bitmap
drawn on the fringe "wins".  And since the display engine processes
overlays in the order of buffer positions (for left-to-right text),
the overlay that wins is always the last one.

Does this explain what happens?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 25, 2023 at 1:50 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > Obviously, because any other character which is not space
> > will _also_ fix the trailing whitespace diagnostic issued by the
> > checkdoc backend. By definition :-)
>
> OK, let's back up a notch, because I think I've misunderstood you.

Basically, both your suggestions effectively amounted to deleting the
second overlay.  It would indeed solve the problem but only by
destroying the use case.

> the overlay that wins is always the last one.
> Does this explain what happens?

Yes, it does.  Although I would expect that when two overlays
that compete for the same resource (in this case, the
same  portion of the left fringe) the overlay with the highest
priority would win that competition -- not the "last one".

Since I can't change the start and end positions of the overlays,
(because they do quite a bit more than specify a 'before-string
property), I can't choose which overlay is the "last one".  So I
guess this bug is very hard to solve until the "last one wins"
behaviour your describe is changed.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 25 Sep 2023 14:52:07 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> > Does this explain what happens?
> 
> Yes, it does.  Although I would expect that when two overlays
> that compete for the same resource (in this case, the
> same  portion of the left fringe) the overlay with the highest
> priority would win that competition -- not the "last one".

There's no built-in mechanism for the display engine to do that, and
the fact that overlays are processed one by one in the order they are
encountered doesn't help: when processing an overlay the display
engine has no idea there are other overlays in the same screen line
that will "compete" for the fringe display.

> Since I can't change the start and end positions of the overlays,
> (because they do quite a bit more than specify a 'before-string
> property), I can't choose which overlay is the "last one".  So I
> guess this bug is very hard to solve until the "last one wins"
> behaviour your describe is changed.

The 'display' property can have the form

  (when CONDITION . SPEC)

where SPEC is your (left-fringe exclamation-mark compilation-warning)
spec, and CONDITION is evaluated by the display engine and must be
non-nil for SPEC to have effect.  The ELisp manual says:

     You can make any display specification conditional.  To do that,
  package it in another list of the form ‘(when CONDITION . SPEC)’.  Then
  the specification SPEC applies only when CONDITION evaluates to a
  non-‘nil’ value.  During the evaluation, ‘object’ is bound to the string
  or buffer having the conditional ‘display’ property.  ‘position’ and
  ‘buffer-position’ are bound to the position within ‘object’ and the
  buffer position where the ‘display’ property was found, respectively.
  Both positions can be different when ‘object’ is a string.

     Note that CONDITION will only be evaluated when redisplay examines
  the text where this display spec is located, so this feature is best
  suited for conditions that are relatively stable, i.e. yield, for each
  particular buffer position, the same results on every evaluation.  If
  the results change for the same text location, e.g., if the result
  depends on the position of point, then the conditional specification
  might not do what you want, because redisplay examines only those parts
  of buffer text where it has reasons to assume that something changed
  since the last display cycle.

Did you try using that here?

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 25, 2023 at 3:19 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Mon, 25 Sep 2023 14:52:07 +0100
> > Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> >
> > > Does this explain what happens?
> >
> > Yes, it does.  Although I would expect that when two overlays
> > that compete for the same resource (in this case, the
> > same  portion of the left fringe) the overlay with the highest
> > priority would win that competition -- not the "last one".
>
> There's no built-in mechanism for the display engine to do that, and
> the fact that overlays are processed one by one in the order they are
> encountered doesn't help: when processing an overlay the display
> engine has no idea there are other overlays in the same screen line
> that will "compete" for the fringe display.

Could it be changed, as it analyses a line, to keep a record of
which overlays have already touched the left fringe for that line
(and then clear this record as it moves on to other lines).

This would be easiest to implement if the the display doesn't
sometimes consider only a fraction of a line.I don't know if this
holds true.  I assume it does, since otherwise that would mean that
if a line is scrolled near the beginning and truncated, the current
"last one" criteria you described before also doesn't always
hold true.

> Did you try using that here?

No.  It should work, though, but seems fairly complex to implement.
Also sounds a bit slow, though the optimizations maybe solve that.

IMHO this should be automatic and based on relative overlay
priorities.  It's too heavy a burden to place on overlay
clients.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Mon, 25 Sep 2023 17:55:45 +0100
> Cc: jporterbugs@gmail.com, 66041@debbugs.gnu.org
> 
> > There's no built-in mechanism for the display engine to do that, and
> > the fact that overlays are processed one by one in the order they are
> > encountered doesn't help: when processing an overlay the display
> > engine has no idea there are other overlays in the same screen line
> > that will "compete" for the fringe display.
> 
> Could it be changed, as it analyses a line, to keep a record of
> which overlays have already touched the left fringe for that line
> (and then clear this record as it moves on to other lines).

The display engine does not analyze lines.  It starts where it is told
to start, and goes on, one buffer position at a time, until some other
buffer position, where it was told to stop.  To analyze lines means
scan each line twice, and the apply some (as yet undefined) logic.

> This would be easiest to implement if the the display doesn't
> sometimes consider only a fraction of a line.I don't know if this
> holds true.

That happens in some situations, yes.

> I assume it does, since otherwise that would mean that
> if a line is scrolled near the beginning and truncated, the current
> "last one" criteria you described before also doesn't always
> hold true.

The "last one" means the last overlay processed on a screen line.

> IMHO this should be automatic and based on relative overlay
> priorities.

Overlay priorities are not for overlays that cover disjoint portions
of text.  Also, the same problem will happen when several 'display'
properties that draw on the fringe are put on buffer text, without any
overlays and thus without any priorities to begin with.  So if we want
to implement something like this in the display engine, we'd need to
extend the format of the 'display' property in this case, adding some
kind of "priority" there, or maybe extend the fringe bitmap data
itself.

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[João Távora]

On Mon, Sep 25, 2023 at 6:24 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > I assume it does, since otherwise that would mean that
> > if a line is scrolled near the beginning and truncated, the current
> > "last one" criteria you described before also doesn't always
> > hold true.
>
> The "last one" means the last overlay processed on a screen line.

Ah, ok: good to know.

>
> > IMHO this should be automatic and based on relative overlay
> > priorities.
>
> Overlay priorities are not for overlays that cover disjoint portions
> of text.  Also, the same problem will happen when several 'display'
> properties that draw on the fringe are put on buffer text, without any
> overlays and thus without any priorities to begin with.

True, I forgot about that.

> So if we want
> to implement something like this in the display engine, we'd need to
> extend the format of the 'display' property in this case, adding some
> kind of "priority" there, or maybe extend the fringe bitmap data
> itself.

I think either is acceptable, though the former is slightly better
as it allows one to use the same bitmap data in many priorities.

João

bug#66041: 30.0.50; Should 'flymake-note-echo' inherit from 'compilation-info'?

[Jim Porter]

On 9/25/2023 11:23 AM, João Távora wrote:
> On Mon, Sep 25, 2023 at 6:24 PM Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> So if we want
>> to implement something like this in the display engine, we'd need to
>> extend the format of the 'display' property in this case, adding some
>> kind of "priority" there, or maybe extend the fringe bitmap data
>> itself.
> 
> I think either is acceptable, though the former is slightly better
> as it allows one to use the same bitmap data in many priorities.

This may or may not be helpful, but back in commit b87400c78b04, I added 
the ability to specify the particular bitmap to use for a fringe icon. 
For example, you can set 'bookmark-fringe-mark' to any known fringe 
bitmap, and then you'll see that bitmap in the fringe for your 
bookmarks. In essence, this lets you split the semantic meaning for a 
fringe marker ("this marker means there's a bookmark here") from the 
actual bitmap data ("what it looks like").

There's probably room to expand this convention from bookmarks to other 
parts of Emacs, and maybe Flymake could benefit from it.