bug#30397: Random numbers in grep mode-line

bug#30397: Random numbers in grep mode-line

[Juri Linkov]

What do these seemingly random numbers in the mode-line of the *grep*
buffer mean?  I don't get any logic behind these colored numbers.
They are neither the number of matches nor the number of matched lines.
And why non-zero numbers are always highlighted in red as errors
when there are no errors in the grep output?  What was the goal
of this feature and where it is documented?

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> What do these seemingly random numbers in the mode-line of the *grep*
> buffer mean?  I don't get any logic behind these colored numbers.
> They are neither the number of matches nor the number of matched lines.
> And why non-zero numbers are always highlighted in red as errors
> when there are no errors in the grep output?  What was the goal
> of this feature and where it is documented?

+1.

A mouseover tooltip says this:

1. Number of errors so far
2. Number of warnings so far
3. Number of informational messages so far

But `C-h m' says nothing about this (it should).

And clicking `compilation-mode' (the parent) in the `C-h m' output
shows that mode's doc, but it too says nothing about this.

bug#30397: Random numbers in grep mode-line

[Noam Postavsky]

On Thu, Feb 8, 2018 at 4:32 PM, Juri Linkov <juri@linkov.net> wrote:
> What was the goal of this feature and where it is documented?

`(emacs) Compilation' (and similar in etc/NEWS):

      While compilation proceeds, the mode line is updated to show the
    number of errors, warnings, and informational messages that have been
    seen so far.

Perhaps it needs some adjustment for grep.

Original report: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25354

bug#30397: Random numbers in grep mode-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 08 Feb 2018 23:32:51 +0200
> 
> What do these seemingly random numbers in the mode-line of the *grep*
> buffer mean?  I don't get any logic behind these colored numbers.
> They are neither the number of matches nor the number of matched lines.
> And why non-zero numbers are always highlighted in red as errors
> when there are no errors in the grep output?  What was the goal
> of this feature and where it is documented?

From NEWS:

  ** Compilation mode
  [...]
  *** The number of errors, warnings, and informational messages is now
  displayed in the mode line.  These are updated as compilation
  proceeds.

Also mentioned in the user manual, in "Compilation":

     While compilation proceeds, the mode line is updated to show the
  number of errors, warnings, and informational messages that have been
  seen so far.

I've now added a similar text in "Grep".

Maybe we should modify the display for Grep, e.g. show only one
number, and use a distinct face that doesn't display as red by
default.

bug#30397: Random numbers in grep mode-line

[Eli Zaretskii]

> Date: Thu, 8 Feb 2018 13:48:55 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> But `C-h m' says nothing about this (it should).
> 
> And clicking `compilation-mode' (the parent) in the `C-h m' output
> shows that mode's doc, but it too says nothing about this.

I don't see modes whose "C-h m" tells anything about mode-line
indicators.  Do you?

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> > But `C-h m' says nothing about this (it should).
> >
> > And clicking `compilation-mode' (the parent) in the `C-h m' output
> > shows that mode's doc, but it too says nothing about this.
> 
> I don't see modes whose "C-h m" tells anything about mode-line
> indicators.  Do you?

No. And?

How many modes do you see that have mode-line info
that needs explanation?  There might well be some
others - in that case, feel free to file bugs for
those too.

The point is that if something the mode does is not
obvious from the UI, it might help for the mode doc
to say something about it.

For `grep', in particular, these numbers, even with
their mouseover tooltips, may leave users scratching
their heads.

Juri is hardly a novice, to either Emacs or `grep'.
I'm not that much of a novice either.  We both,
apparently, feel that this mode-line indication is
not sufficiently self-expanatory.  There may even
be some question (e.g. for `grep') how useful it is.

If you agree that better help about this would be
in order, where would you suggest putting that help,
if not on `C-h m'?  (If you don't agree that
improvement is needed then why ask about putting it
on `C-h m'?)

bug#30397: Random numbers in grep mode-line

[Eli Zaretskii]

> Date: Fri, 9 Feb 2018 07:27:55 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: juri@linkov.net, 30397@debbugs.gnu.org
> 
> > > But `C-h m' says nothing about this (it should).
> > >
> > > And clicking `compilation-mode' (the parent) in the `C-h m' output
> > > shows that mode's doc, but it too says nothing about this.
> > 
> > I don't see modes whose "C-h m" tells anything about mode-line
> > indicators.  Do you?
> 
> No. And?
> 
> How many modes do you see that have mode-line info
> that needs explanation?

From my POV, almost all of those which put there something that is not
just the mode's (abbreviated) name.

> There might well be some others - in that case, feel free to file
> bugs for those too.

Filing a bug doesn't solve the problem.

> If you agree that better help about this would be
> in order, where would you suggest putting that help,
> if not on `C-h m'?

Our current practice is to describe that in the manual.  If we decide
to add that to "C-h m", we should do that for all the modes.  We
should also consider the discoverability: "C-h m" is not where I would
look for documentation of a mode, only for its keybindings.

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> Maybe we should modify the display for Grep, e.g. show only one
> number, and use a distinct face that doesn't display as red by
> default.

Probably, yes.  It is especially for `grep' that the
indications are not so clear or helpful (the last two).

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> > If you agree that better help about this would be
> > in order, where would you suggest putting that help,
> > if not on `C-h m'?
> 
> Our current practice is to describe that in the manual.

That's OK by me.  Somewhere is better than nowhere.  The
manual is better than just NEWS.

> If we decide to add that to "C-h m", we should do that
> for all the modes.

That doesn't follow.

We sometimes describe some keys in `C-h m' output.  That
doesn't mean we should describe all keys in the mode map.

We should describe what needs to be described - in
particular, (1) things that might be the least obvious,
clear or easy to discover and (2) things that might be
the most important to know.

> We should also consider the discoverability:

Definitely.  That's why NEWS doesn't suffice.

The best discoverability for something in the mode-line
is mouseover tooltip info - right there.  The second best
is clicking that thing in the mode line - right there.

For mode indications in the mode-line, I do think that
`C-h m' would not be a bad place to describe them, when
the info is needed (i.e., when they are not clear on
their own and you cannot get more info about them from
the mode-line itself - see previous).

> "C-h m" is not where I would look for documentation
> of a mode, only for its keybindings.

Only its key bindings?  Not I.  To me, `C-h m' should
give an overview of the mode: what it's for etc.  If
`C-h m' for some mode just lists a few key bindings
then I'm disappointed and want more info.

`C-h m' is sometimes (often? typically?) the doc
string of the mode function.  As such, like any
command doc string, it should describe the command.

I mention these general thoughts about `C-h m' because
you brought up the general question.  I have no problem
with this bug being fixed by adding description in the
manual.  And perhaps modifiying what users see for
`grep' in the UI.

bug#30397: Random numbers in grep mode-line

[Juri Linkov]

>> What was the goal of this feature and where it is documented?
>
> `(emacs) Compilation' (and similar in etc/NEWS):
>
>       While compilation proceeds, the mode line is updated to show the
>     number of errors, warnings, and informational messages that have been
>     seen so far.
>
> Perhaps it needs some adjustment for grep.
>
> Original report: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25354

Yes, some adjustment is needed for grep.  That reminded me
about two unclosed feature requests: bug#13417 and bug#14017
that proposed to display these numbers also at the bottom of
output buffers.  But the showstopper was to decide on the
final format of such messages.  Although this looks good:

  Grep finished with 42 matches in 5 lines at Thu Jul 21 15:02:15

Than the mode-line will display two numbers: the number of matches
and the number of matching lines (in green).

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> Yes, some adjustment is needed for grep.  That reminded me
> about two unclosed feature requests: bug#13417 and bug#14017
> that proposed to display these numbers also at the bottom of
> output buffers.  But the showstopper was to decide on the
> final format of such messages.  Although this looks good:
> 
>   Grep finished with 42 matches in 5 lines at Thu Jul 21 15:02:15
> 
> Than the mode-line will display two numbers: the number of matches
> and the number of matching lines (in green).

Is the total number of lines (in the search space) also
available?  If so, would that be useful?  Maybe something
like this?

Grep finished at Thu Jul 21 15:02:15 - 42 matches in 5/113 lines 
                                                      ^^^^

bug#30397: Random numbers in grep mode-line

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >       While compilation proceeds, the mode line is updated to show the
  > >     number of errors, warnings, and informational messages that have been
  > >     seen so far.

That has a gratuitous passive verb.
This text avoids that and is clearer in other ways.

======================================================================
  The mode line displays and updates the number of errors, number of
warnings, and number of informational messages emitted by compilation.
======================================================================

Would someone please install this and ack?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.

bug#30397: Random numbers in grep mode-line

[Juri Linkov]

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

>> Yes, some adjustment is needed for grep.  That reminded me
>> about two unclosed feature requests: bug#13417 and bug#14017
>> that proposed to display these numbers also at the bottom of
>> output buffers.  But the showstopper was to decide on the
>> final format of such messages.  Although this looks good:
>> 
>>   Grep finished with 42 matches in 5 lines at Thu Jul 21 15:02:15
>> 
>> Than the mode-line will display two numbers: the number of matches
>> and the number of matching lines (in green).
>
> Is the total number of lines (in the search space) also
> available?  If so, would that be useful?  Maybe something
> like this?
>
> Grep finished at Thu Jul 21 15:02:15 - 42 matches in 5/113 lines 
>                                                       ^^^^

Unfortunately the total number of lines is not available.
There is even problems with getting the right number of matches.
When grep doesn't highlight matches, we can't count them.

Another problem is that grep matches to be printed at the end of the
grep buffer can't be counted in grep-regexp-alist because it is
based on font-lock which is invoked at unpredictable times
when grep process is already finished.  This leaves only one way
to count matches in grep-filter in this patch.

For the same reason, printing the number of compilation errors at
the end of the compilation buffer can't be implemented for bug#13417.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grep-matches-mode-line.patch --]
[-- Type: text/x-diff, Size: 3990 bytes --]

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 9ce4ff8..23de8aa 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -425,6 +425,14 @@ grep-match-face
 (defvar grep-context-face 'shadow
   "Face name to use for grep context lines.")
 
+(defvar grep-num-matches-found 0)
+
+(defconst grep-mode-line-matches
+  `(" [" (:propertize (:eval (int-to-string grep-num-matches-found))
+                      face ,grep-hit-face
+                      help-echo "Number of matches so far")
+    "]"))
+
 (defvar grep-mode-font-lock-keywords
    '(;; Command output lines.
      (": \\(.+\\): \\(?:Permission denied\\|No such \\(?:file or directory\\|device or address\\)\\)$"
@@ -432,7 +440,7 @@ grep-mode-font-lock-keywords
      ;; remove match from grep-regexp-alist before fontifying
      ("^Grep[/a-zA-z]* started.*"
       (0 '(face nil compilation-message nil help-echo nil mouse-face nil) t))
-     ("^Grep[/a-zA-z]* finished \\(?:(\\(matches found\\))\\|with \\(no matches found\\)\\).*"
+     ("^Grep[/a-zA-z]* finished \\(?:with \\(\\(?:[0-9]+ \\)?matches found\\)\\|with \\(no matches found\\)\\).*"
       (0 '(face nil compilation-message nil help-echo nil mouse-face nil) t)
       (1 compilation-info-face nil t)
       (2 compilation-warning-face nil t))
@@ -503,21 +511,28 @@ grep-process-setup
     (setenv "GREP_COLOR" "01;31")
     ;; GREP_COLORS is used in GNU grep 2.5.2 and later versions
     (setenv "GREP_COLORS" "mt=01;31:fn=:ln=:bn=:se=:sl=:cx=:ne"))
+  (setq-local grep-num-matches-found 0)
   (set (make-local-variable 'compilation-exit-message-function)
-       (lambda (status code msg)
-	 (if (eq status 'exit)
-	     ;; This relies on the fact that `compilation-start'
-	     ;; sets buffer-modified to nil before running the command,
-	     ;; so the buffer is still unmodified if there is no output.
-	     (cond ((and (zerop code) (buffer-modified-p))
-		    '("finished (matches found)\n" . "matched"))
-		   ((not (buffer-modified-p))
-		    '("finished with no matches found\n" . "no match"))
-		   (t
-		    (cons msg code)))
-	   (cons msg code))))
+       'grep-exit-message)
   (run-hooks 'grep-setup-hook))
 
+(defun grep-exit-message (status code msg)
+  "Return a status message for grep results."
+  (if (eq status 'exit)
+      ;; This relies on the fact that `compilation-start'
+      ;; sets buffer-modified to nil before running the command,
+      ;; so the buffer is still unmodified if there is no output.
+      (cond ((and (zerop code) (buffer-modified-p))
+	     (if (> grep-num-matches-found 0)
+                 (cons (format "finished with %d matches found\n" grep-num-matches-found)
+                       "matched")
+               '("finished with matches found\n" . "matched")))
+	    ((not (buffer-modified-p))
+	     '("finished with no matches found\n" . "no match"))
+	    (t
+	     (cons msg code)))
+    (cons msg code)))
+
 (defun grep-filter ()
   "Handle match highlighting escape sequences inserted by the grep process.
 This function is called from `compilation-filter-hook'."
@@ -535,7 +550,8 @@ grep-filter
         (while (re-search-forward "\033\\[0?1;31m\\(.*?\\)\033\\[[0-9]*m" end 1)
           (replace-match (propertize (match-string 1)
                                      'face nil 'font-lock-face grep-match-face)
-                         t t))
+                         t t)
+          (cl-incf grep-num-matches-found))
         ;; Delete all remaining escape sequences
         (goto-char beg)
         (while (re-search-forward "\033\\[[0-9;]*[mK]" end 1)
@@ -775,6 +791,8 @@ grep-mode
        grep-hit-face)
   (set (make-local-variable 'compilation-error-regexp-alist)
        grep-regexp-alist)
+  (set (make-local-variable 'compilation-mode-line-errors)
+       grep-mode-line-matches)
   ;; compilation-directory-matcher can't be nil, so we set it to a regexp that
   ;; can never match.
   (set (make-local-variable 'compilation-directory-matcher) '("\\`a\\`"))

bug#30397: Random numbers in grep mode-line

[Drew Adams]

> > Is the total number of lines (in the search space) also
> > available?  If so, would that be useful?  Maybe something
> > like this?
> >
> > Grep finished at Thu Jul 21 15:02:15 - 42 matches in 5/113 lines
> >                                                       ^^^^
> 
> Unfortunately the total number of lines is not available.
> There is even problems with getting the right number of matches.
> When grep doesn't highlight matches, we can't count them.

In that case, the explanation/description should say that
it is the number of matches highlighted, not the number
of matches.

> Another problem is that grep matches to be printed at the end of the
> grep buffer can't be counted in grep-regexp-alist because it is
> based on font-lock which is invoked at unpredictable times
> when grep process is already finished.  This leaves only one way
> to count matches in grep-filter in this patch.
> 
> For the same reason, printing the number of compilation errors at
> the end of the compilation buffer can't be implemented for bug#13417.

If the numbers shown depend on font-lock and are not
necessarily accurate then the doc should make that
clear.

bug#30397: Random numbers in grep mode-line

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 11 Feb 2018 23:40:05 +0200
> Cc: 30397@debbugs.gnu.org, Noam Postavsky <npostavs@users.sourceforge.net>
> 
> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
> index 9ce4ff8..23de8aa 100644
> --- a/lisp/progmodes/grep.el
> +++ b/lisp/progmodes/grep.el

Thanks, this LGTM for the emacs-26 branch.

bug#30397: Random numbers in grep mode-line

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Sun, 11 Feb 2018 15:45:04 -0500
> Cc: 30397@debbugs.gnu.org, npostavs@users.sourceforge.net
> 
>   > >       While compilation proceeds, the mode line is updated to show the
>   > >     number of errors, warnings, and informational messages that have been
>   > >     seen so far.
> 
> That has a gratuitous passive verb.
> This text avoids that and is clearer in other ways.
> 
> ======================================================================
>   The mode line displays and updates the number of errors, number of
> warnings, and number of informational messages emitted by compilation.
> ======================================================================
> 
> Would someone please install this and ack?

Ack.

bug#30397: Random numbers in grep mode-line

[Juri Linkov]

tags 30397 fixed
close 30397 26.1
tags 14017 fixed
close 14017 26.1
tags 13417 wontfix
close 13417
quit

>> diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
>> index 9ce4ff8..23de8aa 100644
>> --- a/lisp/progmodes/grep.el
>> +++ b/lisp/progmodes/grep.el
>
> Thanks, this LGTM for the emacs-26 branch.

Pushed to the emacs-26 branch.