Flymake, compilation-mode lighters very noisy

Flymake, compilation-mode lighters very noisy

[Yuri Khan]

Hello,

I was fiddling with my Python development environment to start using
python-language-server, and settled on Eglot as a novice-friendly LSP
client.

Now, one side effect of Eglot is that it enables Flymake unconditionally.

Not that I mind, no; actually, I find it very convenient that
potential bugs are highlighted as soon as I create them.

What I do mind, though, is these two menacing eyes staring at me from
the mode line.

    … (Python yas WS SP Flymake[0 0] Projectile[…] ARev ElDoc)
                               ↗   ↖
                bold bright red     bold yellow

(seriously though, I just find bright/high-constrast spots in my
peripheral vision distracting.)

Compilation-mode has the same issue, with as many as four eyes:

    … (Compilation:exit [0] [0 0 0])
                  ↗     ↗   ↗  ↑  ↖
                  green   red  |   green
                             yellow

I understand the motivation: provide instant error, warning and info
counters, and use color coding to avoid having to explain which
counter is for which severity level.

However, I wonder if zero counters could be skipped in the mode line.
After all, they have nothing to say; they shouldn’t be yelling about
that. IMHO.

In the case of compilation-mode, I can redefine the
‘compilation-mode-line-errors’ variable easily enough; it’s just a
mode line fragment referencing a few variables.

Flymake, though, delegates the work to the function
‘flymake--mode-line-format’, which is pretty involved (110 lines of
code). Not much I can do with that, as a user, except copy-pasting
that function into my init.el with a minor change.

Thoughts?

Re: Flymake, compilation-mode lighters very noisy

[João Távora]

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

>
>
>
> Flymake, though, delegates the work to the function
> ‘flymake--mode-line-format’, which is pretty involved (110 lines of
> code). Not much I can do with that, as a user, except copy-pasting
> that function into my init.el with a minor change.
>
> Thoughts?
>

Flymake is reasonably new and untested. I'm glad I made eglot a gateway to
testing it and suggesting improvements.

To your point, propose a way to break up that function. What variable and
what semantics would you like to see? Would it be enough for a variable to
state the minimum severity you would want to always see, even with zero
occurance? In your case you would set it somewhere above "error".... If you
read closely you'll find it's now at "warning", though indeed hardcoded.

And let me know over at GitHub how eglot is working out, too...

João

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

Re: Flymake, compilation-mode lighters very noisy

[Yuri Khan]

On Wed, Nov 21, 2018 at 3:16 AM João Távora <joaotavora@gmail.com> wrote:

>> Flymake, though, delegates the work to the function
>> ‘flymake--mode-line-format’, which is pretty involved (110 lines of
>> code). Not much I can do with that, as a user, except copy-pasting
>> that function into my init.el with a minor change.
>
> To your point, propose a way to break up that function. What variable and what semantics would you like to see? Would it be enough for a variable to state the minimum severity you would want to always see, even with zero occurance? In your case you would set it somewhere above "error".... If you read closely you'll find it's now at "warning", though indeed hardcoded.

Well, as I said before, the function is “pretty involved”. That is to
say, I do not immediately understand all the things it is doing. You
are asking me to read and comprehend it. Hm, okay.


* I see it first calls a few functions to get the current state and
binds their results to local variables. Gonna ignore that for a
moment. It also does some data reshaping (7 lines) from the
buffer-local variable ‘flymake--backend-state’ into local variable
‘diags-by-type’. That could be a helper function.

* I see it dynamically builds a mode line construct, which is always a list.

* Its first element is always the minor mode name, propertized to show
some statistics on mouse hover, show the mode menu on click, and show
help on middle click. This part takes 17 lines. The only dynamic
dependencies of this part are (length known), (length running), and
(length disabled).

* The second element is nil in the normal case, but may turn into a
status item when there are no known backends, or some backends are
still running, or all backends are disabled. This part consists of a
data collection part (12 lines, binds three variables in a pcase-let)
and a conditionally applied mode line construct template (10 lines,
depends on ind, face, explain variables bound just above).

* The last part of the function (56 lines) conditionally generates a
list of "[", followed by space-separated counters by type in
descending severity level order, followed by "]". A counter for a type
is skipped if it has no diagnostics and its severity is lower than
:warning. Each counter is a decimal number representation of the
number of diagnostics of a particular severity, propertized with the
face corresponding to the severity level, a constant mouse-face, and a
dynamically-generated keymap. The mode line construct for each counter
essentially depends on: (length diags), severity, and type.


So I guess what I would like to see is a better separation of logic
from presentation. Concretely, what I would do is:

1. Extract many variables, each of which represents a single mode line
construct used in the Flymake mode line indicator. Make them reference
any necessary data passed as values of predefined symbols.

2. Extract constants such as the Flymake indicator’s keymap as
defconsts, and near-constants such as each type’s counter’s keymap as
defuns.

3. Move the decision to display or skip each element into its
corresponding data structure, possibly using the (SYMBOL THEN ELSE)
mode line construct.

4. The flymake-mode-line-format will be left with all the calculations
and a bunch of (format-mode-line) calls using the variables extracted
in (1), wrapped in let forms binding the necessary dynamic values to
symbols for variables to use.


A little code to demonstrate:

;;..5...10....5...20....5...30....5...40....5...50....5...60....5...70..
;; A user could elide zero counts from here
;; by wrapping each (format …) into a (when …)
(defvar flymake-overall-indicator-help-echo-format
  '((:eval (format "%d known backends\n" flymake-known-backend-count))
    (:eval (format "%d running backends\n" flymake-running-backend-count))
    (:eval (format "%d disabled backends\n" flymake-disabled-backend-count))
    ("mouse-1: Display minor mode menu\n")
    ("mouse-2: Show help for minor mode")))

(defvar flymake-overall-indicator-format
  '(:propertize " Flymake"
                mouse-face mode-line-highlight
                help-echo flymake-overall-indicator-help-echo
                keymap flymake-overall-indicator-keymap))

(defun flymake--mode-line-format ()
  (let* ((known …)
         …more variables…)
    `(,(let* ((flymake-known-backend-count (length known))
              (flymake-running-backend-count (length running))
              (flymake-disabled-backend-count (length disabled))
              (flymake-overall-indicator-help-echo
               (format-mode-line flymake-overall-indicator-help-echo-format)))
         (format-mode-line flymake-overall-indicator))
      …more mode line constructs…)))

This way, for each element, the user gets a well-defined way to
customize when and how it appears, without having to copy the whole
function.


Now as to why a simple severity level variable will not cut it: The
color coding is helpful, but people with different accessibility needs
will want to replace it with different things. A color-blind user may
want to add letters, e.g. 0e 0w. A user with ADD-like symptoms will
prefer no color, with or without letters. A visually inclined user may
want to replace the string “Flymake” with a Unicode check mark/info
sign/warning sign/cross. A small screen user will want to see only one
counter with the most severity, and hide warnings if there are errors.

Re: Flymake, compilation-mode lighters very noisy

[João Távora]

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

This is too long for me to read right now, sorry.

The simple variable flymake-always-show-indicators-min-severity-level would
fix the problem you reported yesterday, and it would be trivial to
implement, but you seem to be suggesting much more.

Propose those changes in a patch or a scratch branch! Or, alternatively,
propose only the new customization variable/interfaces you would like to
see added, along with docstrings, and we can deal with the implementation
later after we settle on the user interface.

Thanks,
João




On Wed, Nov 21, 2018, 09:55 Yuri Khan <yurivkhan@gmail.com wrote:

> On Wed, Nov 21, 2018 at 3:16 AM João Távora <joaotavora@gmail.com> wrote:
>
> >> Flymake, though, delegates the work to the function
> >> ‘flymake--mode-line-format’, which is pretty involved (110 lines of
> >> code). Not much I can do with that, as a user, except copy-pasting
> >> that function into my init.el with a minor change.
> >
> > To your point, propose a way to break up that function. What variable
> and what semantics would you like to see? Would it be enough for a variable
> to state the minimum severity you would want to always see, even with zero
> occurance? In your case you would set it somewhere above "error".... If you
> read closely you'll find it's now at "warning", though indeed hardcoded.
>
> Well, as I said before, the function is “pretty involved”. That is to
> say, I do not immediately understand all the things it is doing. You
> are asking me to read and comprehend it. Hm, okay.
>
>
> * I see it first calls a few functions to get the current state and
> binds their results to local variables. Gonna ignore that for a
> moment. It also does some data reshaping (7 lines) from the
> buffer-local variable ‘flymake--backend-state’ into local variable
> ‘diags-by-type’. That could be a helper function.
>
> * I see it dynamically builds a mode line construct, which is always a
> list.
>
> * Its first element is always the minor mode name, propertized to show
> some statistics on mouse hover, show the mode menu on click, and show
> help on middle click. This part takes 17 lines. The only dynamic
> dependencies of this part are (length known), (length running), and
> (length disabled).
>
> * The second element is nil in the normal case, but may turn into a
> status item when there are no known backends, or some backends are
> still running, or all backends are disabled. This part consists of a
> data collection part (12 lines, binds three variables in a pcase-let)
> and a conditionally applied mode line construct template (10 lines,
> depends on ind, face, explain variables bound just above).
>
> * The last part of the function (56 lines) conditionally generates a
> list of "[", followed by space-separated counters by type in
> descending severity level order, followed by "]". A counter for a type
> is skipped if it has no diagnostics and its severity is lower than
> :warning. Each counter is a decimal number representation of the
> number of diagnostics of a particular severity, propertized with the
> face corresponding to the severity level, a constant mouse-face, and a
> dynamically-generated keymap. The mode line construct for each counter
> essentially depends on: (length diags), severity, and type.
>
>
> So I guess what I would like to see is a better separation of logic
> from presentation. Concretely, what I would do is:
>
> 1. Extract many variables, each of which represents a single mode line
> construct used in the Flymake mode line indicator. Make them reference
> any necessary data passed as values of predefined symbols.
>
> 2. Extract constants such as the Flymake indicator’s keymap as
> defconsts, and near-constants such as each type’s counter’s keymap as
> defuns.
>
> 3. Move the decision to display or skip each element into its
> corresponding data structure, possibly using the (SYMBOL THEN ELSE)
> mode line construct.
>
> 4. The flymake-mode-line-format will be left with all the calculations
> and a bunch of (format-mode-line) calls using the variables extracted
> in (1), wrapped in let forms binding the necessary dynamic values to
> symbols for variables to use.
>
>
> A little code to demonstrate:
>
> ;;..5...10....5...20....5...30....5...40....5...50....5...60....5...70..
> ;; A user could elide zero counts from here
> ;; by wrapping each (format …) into a (when …)
> (defvar flymake-overall-indicator-help-echo-format
>   '((:eval (format "%d known backends\n" flymake-known-backend-count))
>     (:eval (format "%d running backends\n" flymake-running-backend-count))
>     (:eval (format "%d disabled backends\n"
> flymake-disabled-backend-count))
>     ("mouse-1: Display minor mode menu\n")
>     ("mouse-2: Show help for minor mode")))
>
> (defvar flymake-overall-indicator-format
>   '(:propertize " Flymake"
>                 mouse-face mode-line-highlight
>                 help-echo flymake-overall-indicator-help-echo
>                 keymap flymake-overall-indicator-keymap))
>
> (defun flymake--mode-line-format ()
>   (let* ((known …)
>          …more variables…)
>     `(,(let* ((flymake-known-backend-count (length known))
>               (flymake-running-backend-count (length running))
>               (flymake-disabled-backend-count (length disabled))
>               (flymake-overall-indicator-help-echo
>                (format-mode-line
> flymake-overall-indicator-help-echo-format)))
>          (format-mode-line flymake-overall-indicator))
>       …more mode line constructs…)))
>
> This way, for each element, the user gets a well-defined way to
> customize when and how it appears, without having to copy the whole
> function.
>
>
> Now as to why a simple severity level variable will not cut it: The
> color coding is helpful, but people with different accessibility needs
> will want to replace it with different things. A color-blind user may
> want to add letters, e.g. 0e 0w. A user with ADD-like symptoms will
> prefer no color, with or without letters. A visually inclined user may
> want to replace the string “Flymake” with a Unicode check mark/info
> sign/warning sign/cross. A small screen user will want to see only one
> counter with the most severity, and hide warnings if there are errors.
>

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

Re: Flymake, compilation-mode lighters very noisy

[João Távora]

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

By the way, I only asked you to propose a break up of that function because
you talked about copying it to your init and tweaking it. I assumed those
tweaks would not be random... :)

Anyway I didn't mean to force you into reading a boring implementation
detail, sorry.

João

On Wed, Nov 21, 2018, 10:38 João Távora <joaotavora@gmail.com wrote:

> This is too long for me to read right now, sorry.
>
> The simple variable flymake-always-show-indicators-min-severity-level
> would fix the problem you reported yesterday, and it would be trivial to
> implement, but you seem to be suggesting much more.
>
> Propose those changes in a patch or a scratch branch! Or, alternatively,
> propose only the new customization variable/interfaces you would like to
> see added, along with docstrings, and we can deal with the implementation
> later after we settle on the user interface.
>
> Thanks,
> João
>
>
>
>
> On Wed, Nov 21, 2018, 09:55 Yuri Khan <yurivkhan@gmail.com wrote:
>
>> On Wed, Nov 21, 2018 at 3:16 AM João Távora <joaotavora@gmail.com> wrote:
>>
>> >> Flymake, though, delegates the work to the function
>> >> ‘flymake--mode-line-format’, which is pretty involved (110 lines of
>> >> code). Not much I can do with that, as a user, except copy-pasting
>> >> that function into my init.el with a minor change.
>> >
>> > To your point, propose a way to break up that function. What variable
>> and what semantics would you like to see? Would it be enough for a variable
>> to state the minimum severity you would want to always see, even with zero
>> occurance? In your case you would set it somewhere above "error".... If you
>> read closely you'll find it's now at "warning", though indeed hardcoded.
>>
>> Well, as I said before, the function is “pretty involved”. That is to
>> say, I do not immediately understand all the things it is doing. You
>> are asking me to read and comprehend it. Hm, okay.
>>
>>
>> * I see it first calls a few functions to get the current state and
>> binds their results to local variables. Gonna ignore that for a
>> moment. It also does some data reshaping (7 lines) from the
>> buffer-local variable ‘flymake--backend-state’ into local variable
>> ‘diags-by-type’. That could be a helper function.
>>
>> * I see it dynamically builds a mode line construct, which is always a
>> list.
>>
>> * Its first element is always the minor mode name, propertized to show
>> some statistics on mouse hover, show the mode menu on click, and show
>> help on middle click. This part takes 17 lines. The only dynamic
>> dependencies of this part are (length known), (length running), and
>> (length disabled).
>>
>> * The second element is nil in the normal case, but may turn into a
>> status item when there are no known backends, or some backends are
>> still running, or all backends are disabled. This part consists of a
>> data collection part (12 lines, binds three variables in a pcase-let)
>> and a conditionally applied mode line construct template (10 lines,
>> depends on ind, face, explain variables bound just above).
>>
>> * The last part of the function (56 lines) conditionally generates a
>> list of "[", followed by space-separated counters by type in
>> descending severity level order, followed by "]". A counter for a type
>> is skipped if it has no diagnostics and its severity is lower than
>> :warning. Each counter is a decimal number representation of the
>> number of diagnostics of a particular severity, propertized with the
>> face corresponding to the severity level, a constant mouse-face, and a
>> dynamically-generated keymap. The mode line construct for each counter
>> essentially depends on: (length diags), severity, and type.
>>
>>
>> So I guess what I would like to see is a better separation of logic
>> from presentation. Concretely, what I would do is:
>>
>> 1. Extract many variables, each of which represents a single mode line
>> construct used in the Flymake mode line indicator. Make them reference
>> any necessary data passed as values of predefined symbols.
>>
>> 2. Extract constants such as the Flymake indicator’s keymap as
>> defconsts, and near-constants such as each type’s counter’s keymap as
>> defuns.
>>
>> 3. Move the decision to display or skip each element into its
>> corresponding data structure, possibly using the (SYMBOL THEN ELSE)
>> mode line construct.
>>
>> 4. The flymake-mode-line-format will be left with all the calculations
>> and a bunch of (format-mode-line) calls using the variables extracted
>> in (1), wrapped in let forms binding the necessary dynamic values to
>> symbols for variables to use.
>>
>>
>> A little code to demonstrate:
>>
>> ;;..5...10....5...20....5...30....5...40....5...50....5...60....5...70..
>> ;; A user could elide zero counts from here
>> ;; by wrapping each (format …) into a (when …)
>> (defvar flymake-overall-indicator-help-echo-format
>>   '((:eval (format "%d known backends\n" flymake-known-backend-count))
>>     (:eval (format "%d running backends\n" flymake-running-backend-count))
>>     (:eval (format "%d disabled backends\n"
>> flymake-disabled-backend-count))
>>     ("mouse-1: Display minor mode menu\n")
>>     ("mouse-2: Show help for minor mode")))
>>
>> (defvar flymake-overall-indicator-format
>>   '(:propertize " Flymake"
>>                 mouse-face mode-line-highlight
>>                 help-echo flymake-overall-indicator-help-echo
>>                 keymap flymake-overall-indicator-keymap))
>>
>> (defun flymake--mode-line-format ()
>>   (let* ((known …)
>>          …more variables…)
>>     `(,(let* ((flymake-known-backend-count (length known))
>>               (flymake-running-backend-count (length running))
>>               (flymake-disabled-backend-count (length disabled))
>>               (flymake-overall-indicator-help-echo
>>                (format-mode-line
>> flymake-overall-indicator-help-echo-format)))
>>          (format-mode-line flymake-overall-indicator))
>>       …more mode line constructs…)))
>>
>> This way, for each element, the user gets a well-defined way to
>> customize when and how it appears, without having to copy the whole
>> function.
>>
>>
>> Now as to why a simple severity level variable will not cut it: The
>> color coding is helpful, but people with different accessibility needs
>> will want to replace it with different things. A color-blind user may
>> want to add letters, e.g. 0e 0w. A user with ADD-like symptoms will
>> prefer no color, with or without letters. A visually inclined user may
>> want to replace the string “Flymake” with a Unicode check mark/info
>> sign/warning sign/cross. A small screen user will want to see only one
>> counter with the most severity, and hide warnings if there are errors.
>>
>

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

Re: Flymake, compilation-mode lighters very noisy

[Yuri Khan]

On Wed, Nov 21, 2018 at 5:39 PM João Távora <joaotavora@gmail.com> wrote:
>
> This is too long for me to read right now, sorry.

Please take your time. No pressure.

> The simple variable flymake-always-show-indicators-min-severity-level would fix the problem you reported yesterday, and it would be trivial to implement, but you seem to be suggesting much more.

I am suggesting a massive logic:presentation split-up, so that not
only my problem would be fixed but also similar problems for different
categories of other users, and because such a split-up is generally a
good thing.


> By the way, I only asked you to propose a break up of that function because you talked about copying it to your init and tweaking it. I assumed those tweaks would not be random... :)

Okay, if we are talking about my tweaks only, then I would want to
have a configuration where a red or yellow anything on my mode line
means an actionable alert. That is, only display color if there is
something I *can* and *should* fix immediately.

* First, I would suppress zero counters of all severities. This would
make it possible for me to work on projects that currently have no
errors or warnings and where the policy is to keep errors and warnings
at zero.

* I could get involved in a project that has warnings, and where I
cannot fix them all at the source. Then I would first attempt to see
if I can customize the checker rules to silence those categories of
errors or warnings. If that proved infeasible, then I would have to
live with a non-zero error or warning counter; in that case I would
want to tweak the indicator so that the counters were displayed in a
normal face, not in red and yellow.

…And here I looked at where it gets the faces from and saw I could
just (put 'flymake-error 'mode-line-face nil). Thank you, immediate
problem solved, sorry for the panic :)


> Propose those changes in a patch or a scratch branch! Or, alternatively, propose only the new customization variable/interfaces you would like to see added, along with docstrings, and we can deal with the implementation later after we settle on the user interface.

As someone who has not assigned copyright to FSF, I think I can
currently only propose ideas, and modest amounts of code.

Re: Flymake, compilation-mode lighters very noisy

[João Távora]

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

On Wed, Nov 21, 2018 at 11:36 AM Yuri Khan <yurivkhan@gmail.com> wrote:

> On Wed, Nov 21, 2018 at 5:39 PM João Távora <joaotavora@gmail.com> wrote:
> >
> > This is too long for me to read right now, sorry.
>
> Please take your time. No pressure.
>
> > The simple variable flymake-always-show-indicators-min-severity-level
> would fix the problem you reported yesterday, and it would be trivial to
> implement, but you seem to be suggesting much more.
>
> I am suggesting a massive logic:presentation split-up, so that not
> only my problem would be fixed but also similar problems for different
> categories of other users,


We still need to clear up exactly what situations/problems we would be
fixing...

and because such a split-up is generally a good thing.
>

With emphasis on the "generally" :-).  I'd rather fix concrete problems,
like
the one you brought me originally.  I must admit I don't have much
incentive
in what is potentially an overdesign (for me in particular) of this bit of
functionality, so you must find another "sponsor".

 > By the way, I only asked you to propose a break up of that function
because you talked about copying it to your init and tweaking it. I assumed
those tweaks would not be random... :)

>
> Okay, if we are talking about my tweaks only, then I would want to
> have a configuration where a red or yellow anything on my mode line
> means an actionable alert. That is, only display color if there is
> something I *can* and *should* fix immediately.
>
> * First, I would suppress zero counters of all severities. This would
> make it possible for me to work on projects that currently have no
> errors or warnings and where the policy is to keep errors and warnings
> at zero.
>

OK. So is a flymake-supress-zero-counters variable enough? If set to t,
means
to supress all zero counters. If set to :warning, the default, means to
suppress
zero counters of severity less than warning.  Likewise if set to severity X.
If set to nil, don't supress any zero counters.


> * I could get involved in a project that has warnings, and where I
> cannot fix them all at the source. Then I would first attempt to see
> if I can customize the checker rules to silence those categories of
> errors or warnings. If that proved infeasible, then I would have to
> live with a non-zero error or warning counter; in that case I would
> want to tweak the indicator so that the counters were displayed in a
> normal face, not in red and yellow.
>
> …And here I looked at where it gets the faces from and saw I could
> just (put 'flymake-error 'mode-line-face nil). Thank you, immediate
> problem solved, sorry for the panic :)
>

Yes, I was going to suggest that. That's the standard way to affect
error types.


> > Propose those changes in a patch or a scratch branch! Or, alternatively,
> propose only the new customization variable/interfaces you would like to
> see added, along with docstrings, and we can deal with the implementation
> later after we settle on the user interface.
>
> As someone who has not assigned copyright to FSF, I think I can
> currently only propose ideas, and modest amounts of code.
>

Then start working on that assignment!

João Távora

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

Re: Flymake, compilation-mode lighters very noisy

[Yuri Khan]

On Wed, Nov 21, 2018 at 6:58 PM João Távora <joaotavora@gmail.com> wrote:

> OK. So is a flymake-supress-zero-counters variable enough? If set to t, means
> to supress all zero counters. If set to :warning, the default, means to suppress
> zero counters of severity less than warning.  Likewise if set to severity X.
> If set to nil, don't supress any zero counters.

That looks nice.

Re: Flymake, compilation-mode lighters very noisy

[João Távora]

Yuri Khan <yurivkhan@gmail.com> writes:

> On Wed, Nov 21, 2018 at 6:58 PM João Távora <joaotavora@gmail.com> wrote:
>
>> OK. So is a flymake-supress-zero-counters variable enough? If set to t, means
>> to supress all zero counters. If set to :warning, the default, means to suppress
>> zero counters of severity less than warning.  Likewise if set to severity X.
>> If set to nil, don't supress any zero counters.
>
> That looks nice.

I just pushed this to master (42c1332494220fab8a07f23f33951edbb01d1c81).

João