From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: Flymake, compilation-mode lighters very noisy Date: Wed, 21 Nov 2018 10:42:54 +0000 Message-ID: References: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="0000000000006bdd4a057b2a6aa9" X-Trace: blaine.gmane.org 1542796873 5205 195.159.176.226 (21 Nov 2018 10:41:13 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 21 Nov 2018 10:41:13 +0000 (UTC) Cc: emacs-devel To: Yuri Khan Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Nov 21 11:41:08 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gPPwR-0001Do-Cu for ged-emacs-devel@m.gmane.org; Wed, 21 Nov 2018 11:41:07 +0100 Original-Received: from localhost ([::1]:38242 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPPyX-0000wn-MN for ged-emacs-devel@m.gmane.org; Wed, 21 Nov 2018 05:43:17 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34517) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gPPyQ-0000wV-AI for emacs-devel@gnu.org; Wed, 21 Nov 2018 05:43:12 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gPPyO-0001nz-Jz for emacs-devel@gnu.org; Wed, 21 Nov 2018 05:43:10 -0500 Original-Received: from mail-qt1-x82e.google.com ([2607:f8b0:4864:20::82e]:40697) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gPPyO-0001mM-EV for emacs-devel@gnu.org; Wed, 21 Nov 2018 05:43:08 -0500 Original-Received: by mail-qt1-x82e.google.com with SMTP id k12so3261542qtf.7 for ; Wed, 21 Nov 2018 02:43:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=pXJDr88H6IJvJTP1oGE7pLh7Fj6bUgmBo/44iw2/pQg=; b=lsZ1JtHfpm2zwcpHvzvcgjGZnB4A0j/7ULmpu+D1Fvd7+8QixRbHqTMU0Ci7SN2QkB ulyXB//HKOUWNXJjLMyjfpX8tkyI2DmauvcMrCop7Wv178rUhH6YdBtmT9Mbhnq9Dbqg oQMwHXNJBaV7BiRDEd3dl+eKAam4BErVwv5nYEgEArcG0pv3BgCyM66++Cc8mGxzSPst 1DtRtJVhaiIkhUg1s++kR08vEG3YiQUIrAiRJUiVaA56e2PodJEJ6frQtv8uD7tZJV0b NFsYHQYffMrqeglgFv++QkpS+LShlFZbLCWZgwEUYRtFhdUpfnS1YoVdIlASqg/V5NWV CaIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=pXJDr88H6IJvJTP1oGE7pLh7Fj6bUgmBo/44iw2/pQg=; b=qUBeg0OQ2DdbTeBdwq44EGOmxpl4Jf52/SSVd6pVMGbG17gyQuTb87xKcA4B/yQOl1 NKSY2qf14V9LpQwWAXsoN2c2rESvrPOktRK05MBBHQJGVVio2oIhl66I5vJ7vgjwC6FD 7/4fmUA/9+MMCxeYtAloMooQxfDcBpWWCYdoKSy6Wz1SaMpt1thaJzrMCUlcTqcXhF4B +sBfd17kdyf/prO9bFK18L8UzLujzL3SUJ/exSaX8rDSxye+ptJV7NPYOPxVGFU9siKU +LtXHTwip7TqhktIT2kUgWa1XVj0qnA/f6LKnemw1s1BrpGdSw9AeUUTSdcI57YjJtcR stEw== X-Gm-Message-State: AGRZ1gJjjl0v6d0H/jJsebNLJTcnjbPskLYH1GsDGMvhuLyB/PpoBAMw vncV/t7uNe9Bc4x5cPX0bjLdgXPw8pROfJ1de8IyDCfJ X-Google-Smtp-Source: AJdET5cMq2sl5uIvUFffWabZyhGEGvodBvOBqDjPMpnqBwhK3vH+SInBiM4lP8l51nBb5JAQH8mNZGR87PSxt3N26O0= X-Received: by 2002:ac8:3e91:: with SMTP id y17mr5481278qtf.390.1542796987848; Wed, 21 Nov 2018 02:43:07 -0800 (PST) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::82e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:231276 Archived-At: --0000000000006bdd4a057b2a6aa9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=C3=A3o On Wed, Nov 21, 2018, 10:38 Jo=C3=A3o T=C3=A1vora 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=C3=A3o > > > > > On Wed, Nov 21, 2018, 09:55 Yuri Khan >> On Wed, Nov 21, 2018 at 3:16 AM Jo=C3=A3o T=C3=A1vora wrote: >> >> >> Flymake, though, delegates the work to the function >> >> =E2=80=98flymake--mode-line-format=E2=80=99, 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 varia= ble >> to state the minimum severity you would want to always see, even with ze= ro >> 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 =E2=80=9Cpretty involved=E2=80= =9D. 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 =E2=80=98flymake--backend-state=E2=80=99 into loca= l variable >> =E2=80=98diags-by-type=E2=80=99. 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=E2=80=99s keymap as >> defconsts, and near-constants such as each type=E2=80=99s counter=E2=80= =99s 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 =E2=80=A6) into a (when =E2=80=A6) >> (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 =E2=80=A6) >> =E2=80=A6more variables=E2=80=A6) >> `(,(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)) >> =E2=80=A6more mode line constructs=E2=80=A6))) >> >> 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 =E2=80=9CFlymake=E2=80=9D with a Unicode chec= k 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. >> > --0000000000006bdd4a057b2a6aa9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
By the way, I only asked you to propose a break up of tha= t function because you talked about copying it to your init and tweaking it= . I assumed those tweaks would not be random... :)=C2=A0
<= br>
Anyway I didn't mean to force you into readi= ng a boring implementation detail, sorry.

Jo=C3=A3o

On Wed, Nov 21, 2018, 10:38 Jo=C3=A3o T=C3=A1vora <joaotavora@gmail.com wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px= #ccc solid;padding-left:1ex">
This is too long for me to = read right now, sorry.

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

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

Tha= nks,
Jo=C3=A3o


On Wed, Nov 21, 2018, 09:55 Y= uri Khan <yurivkhan@gmail.com wrote:
On Wed, Nov 21, 2018 at 3:16 AM Jo=C3=A3o T=C3=A1vora <joaotavora@gmail.com> wrote:

>> Flymake, though, delegates the work to the function
>> =E2=80=98flymake--mode-line-format=E2=80=99, which is pretty invol= ved (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 =E2=80=9Cpretty involved=E2=80=9D. = 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 =E2=80=98flymake--backend-state=E2=80=99 into local v= ariable
=E2=80=98diags-by-type=E2=80=99. 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=E2=80=99s keymap as
defconsts, and near-constants such as each type=E2=80=99s counter=E2=80=99s= 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 =E2=80=A6) into a (when =E2=80=A6)
(defvar flymake-overall-indicator-help-echo-format
=C2=A0 '((:eval (format "%d known backends\n" flymake-known-b= ackend-count))
=C2=A0 =C2=A0 (:eval (format "%d running backends\n" flymake-runn= ing-backend-count))
=C2=A0 =C2=A0 (:eval (format "%d disabled backends\n" flymake-dis= abled-backend-count))
=C2=A0 =C2=A0 ("mouse-1: Display minor mode menu\n")
=C2=A0 =C2=A0 ("mouse-2: Show help for minor mode")))

(defvar flymake-overall-indicator-format
=C2=A0 '(:propertize " Flymake"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 mouse-face mode-lin= e-highlight
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 help-echo flymake-o= verall-indicator-help-echo
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 keymap flymake-over= all-indicator-keymap))

(defun flymake--mode-line-format ()
=C2=A0 (let* ((known =E2=80=A6)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=E2=80=A6more variables=E2=80=A6)
=C2=A0 =C2=A0 `(,(let* ((flymake-known-backend-count (length known))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (flymake-running-backend-c= ount (length running))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (flymake-disabled-backend-= count (length disabled))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (flymake-overall-indicator= -help-echo
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(format-mode-line fl= ymake-overall-indicator-help-echo-format)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(format-mode-line flymake-overall-indicat= or))
=C2=A0 =C2=A0 =C2=A0 =E2=80=A6more mode line constructs=E2=80=A6)))

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 =E2=80=9CFlymake=E2=80=9D with a Unicode check m= ark/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.
--0000000000006bdd4a057b2a6aa9--