bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

emacs -Q
 
Load libraries cygwin-mount.el, setup-cygwin.el, to be able to use grep
on Windows.  The libraries are here:
http://www.emacswiki.org/emacs/ElispArea
 
cd to the Emacs Lisp source directory.
 
M-x grep RET grep -nH -e "#" *.el */*.el RET
 
The grep output shows all matches for `#'.
 
C-x C-q to make the grep buffer writable.
 
M-x flush-lines RET ###autoload RET
 
Only the first few lines containing `###autoload' are flushed.
Repeating `flush-lines' flushes a few more.  But all should be flushed
(assuming point is at bob).  Same thing if you do `C-x h' before M-x
flush-lines.
 
This is a regression wrt Emacs 23.3 and older releases.  But the
`flush-lines' code itself seems not to have changed since 23.3.
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-04-04 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/imagesupport/include'
 

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Tue, 5 Apr 2011 12:01:35 -0700
> Cc: 
> 
> emacs -Q
>  
> Load libraries cygwin-mount.el, setup-cygwin.el, to be able to use grep
> on Windows.  The libraries are here:
> http://www.emacswiki.org/emacs/ElispArea
>  
> cd to the Emacs Lisp source directory.
>  
> M-x grep RET grep -nH -e "#" *.el */*.el RET
>  
> The grep output shows all matches for `#'.
>  
> C-x C-q to make the grep buffer writable.
>  
> M-x flush-lines RET ###autoload RET
>  
> Only the first few lines containing `###autoload' are flushed.
> Repeating `flush-lines' flushes a few more.  But all should be flushed
> (assuming point is at bob).  Same thing if you do `C-x h' before M-x
> flush-lines.

Does it work if you copy the entire contents of the Grep buffer to
another buffer?

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

> Does it work if you copy the entire contents of the Grep buffer to
> another buffer?

No, same problem.  But you can try for yourself, given the simple recipe.

After copying to another buffer and trying flush-lines once, I see this (tail
abbreviated):

grep -nH -e "##" *.el */*.el
apropos.el:514:;;;^[[01;31m##^[[00m^[[K#autoload
apropos.el:524:;;;^[[01;31m##^[[00m^[[K#autoload
apropos.el:563:;;;^[[01;31m##^[[00m^[[K#autoload
apropos.el:669:;;;^[[01;31m##^[[00m^[[K#autoload
apropos.el:715:;;;^[[01;31m##^[[00m^[[K#autoload
arc-mode.el:634:;;;^[[01;31m##^[[00m^[[K#autoload
array.el:802:;;;^[[01;31m##^[[00m^[[K#autoload
autoarg.el:91:;;;^[[01;31m##^[[00m^[[K#autoload
autoarg.el:113:;;;^[[01;31m##^[[00m^[[K#autoload
autoinsert.el:318:;;;^[[01;31m##^[[00m^[[K#autoload
autoinsert.el:379:;;;^[[01;31m##^[[00m^[[K#autoload
autoinsert.el:398:;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:117:;;; What's this?: ;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:285:;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:306:;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:315:;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:368:;;;^[[01;31m##^[[00m^[[K#autoload
autorevert.el:377:;;;^[[01;31m##^[[00m^[[K#autoload
...

As you can see, the problem seems to be that the string "###autoload" is not
present as such. Instead, escape characters are inserted between ## and #.
Presumably the first few lines were not like that, so they did get flushed.

When I yanked the buffer text copied from *grep* into the new buffer (foo), the
part of buffer *grep* that had been fontified (or whatever processing is done to
visually replace the escape chars with highlighted text) appeared fontified in
`foo', and thus those lines did have "###autoload" present, so they got flushed.
The remaining lines (many, many), which were not highlighted, did not get
flushed.

So the problem seems to be lazy highlighting. Unhighlighted text interferes with
search etc. because of the escape chars.

I often flush out comment lines etc. from *grep*.  This is a real step backward,
IMO. I hope you will consider it a bug to be fixed.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <8429@debbugs.gnu.org>
> Date: Tue, 5 Apr 2011 13:14:57 -0700
> 
> > Does it work if you copy the entire contents of the Grep buffer to
> > another buffer?
> 
> No, same problem.

Well, as you yourself show, it is not the "same problem".

> But you can try for yourself, given the simple recipe.

I did, that's why I asked.  I wanted to know that we see the same
phenomenon.

(And btw, there's no need to load cygwin-FOO.el to see the problem.
Just "M-x grep RET "#" *.el" is enough.  It is also repeatable on
GNU/Linux.)

> autorevert.el:368:;;;^[[01;31m##^[[00m^[[K#autoload
> autorevert.el:377:;;;^[[01;31m##^[[00m^[[K#autoload
> ...
> 
> As you can see, the problem seems to be that the string "###autoload" is not
> present as such.

Exactly!  Customize grep-highlight-matches to nil, and the problem
goes away.

> Instead, escape characters are inserted between ## and #.

That's Grep in action, when we ask it to highlight matches in its
output.  It does that by inserting ANSI escape sequences.

> I hope you will consider it a bug to be fixed.

Not me, but hopefully someone else.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <8429@debbugs.gnu.org>
> Date: Tue, 5 Apr 2011 13:14:57 -0700
> 
> So the problem seems to be lazy highlighting. Unhighlighted text interferes with
> search etc. because of the escape chars.

Yes, it sounds like flush-lines should retry when it fails to find a
match, after lazy-highlighting the next portion of the buffer.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

> > > Does it work if you copy the entire contents of the Grep buffer to
> > > another buffer?
> > 
> > No, same problem.
> 
> Well, as you yourself show, it is not the "same problem".

Same problem as the one I reported: `flush-lines' removes only the first few
matching lines.

> (And btw, there's no need to load cygwin-FOO.el to see the problem.
> Just "M-x grep RET "#" *.el" is enough.  It is also repeatable on
> GNU/Linux.)

I was clear that this is on Windows.  And on Windows there is no `grep' without
doing something besides emacs -Q.  Hence the recipe for Windows.

> > autorevert.el:368:;;;^[[01;31m##^[[00m^[[K#autoload
> > autorevert.el:377:;;;^[[01;31m##^[[00m^[[K#autoload
> > ...
> > 
> > As you can see, the problem seems to be that the string 
> > "###autoload" is not present as such.
> 
> Exactly!  Customize grep-highlight-matches to nil, and the problem
> goes away.

I hope you are saying that merely in order to lend support for the hypothesis
about the cause of the problem.  Doing that is certainly not a solution to the
problem.

> > Instead, escape characters are inserted between ## and #.
> 
> That's Grep in action, when we ask it to highlight matches in its
> output.  It does that by inserting ANSI escape sequences.

Yes, I know.  It also does that in Emacs 22 and 23, but without the bug.

If I had to guess in ignorance I'd say that it has to do with Emacs 24
highlighting only a screen at a time, instead of the whole buffer.  For the part
of the buffer that gets highlighted (so the escape chars are not apparent) there
is no problem.

> > I hope you will consider it a bug to be fixed.
> 
> Not me, but hopefully someone else.

You don't consider it a bug to be fixed, but you hope someone else will consider
it so?  What's that about?

Turning off highlighting to make the problem go away is not a solution.  This is
a regression and represents a real loss of functionality.

If a better solution is not found, we should at least give users knowledge of
how to make the buffer amenable to `flush-lines' etc.  Give them a command that
does whatever needs to be done.  A single command that makes the buffer editable
and fontifies everything would probably be enough, if it is well enough
advertised (e.g. in the `Grep' menu).  Again, that would be an acceptable
workaround only IF a real solution cannot be found.

I tried `font-lock-fontify-buffer', thinking that might be enough to do the
trick, but it did not.  It is font-locking that removes the escape-char markers,
but I guess the laziness of font-locking is still the problem, even with
`font-lock-fontify-buffer'.  The value of (font-lock-value-in-major-mode
font-lock-support-mode) in *grep* is `jit-lock-mode'.

It is common for users to operate on text in the buffer.  This bug makes the
*grep* buffer much less useful.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

> > So the problem seems to be lazy highlighting. Unhighlighted 
> > text interferes with
> > search etc. because of the escape chars.
> 
> Yes, it sounds like flush-lines should retry when it fails to find a
> match, after lazy-highlighting the next portion of the buffer.

That doesn't sound like the right approach to me.

For one thing, the problem is not limited to `flush-lines'.  Any action on the
buffer text that gets thrown off by the added chars will be affected.  One of
the reasons to run `grep' in Emacs is to have a buffer of text to operate on.

For another thing, it's not clear that changing `flush-lines' in that way would
be appropriate for other use `flush-lines' contexts.

As I said, we might need to opt for letting the user initiate an editing mode.
Until now, `C-x C-q' was enough for that.  But maybe now more is needed.

But that's only if this bug cannot really be fixed in a way that gives back the
pre-regression behavior.

IOW, things worked well in Emacs 22 and 23; what was gained in losing this
behavior?  My guess is that the answer is performance: highlighting is no doubt
much faster because only what's shown gets highlighted.  That is important
(useful), no doubt about it.

If a tradeoff is needed and no better solution can be found, then I'd suggest a
command to make the buffer editable and completely highlighted.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Stefan Monnier]

> If I had to guess in ignorance I'd say that it has to do with Emacs 24
> highlighting only a screen at a time, instead of the whole buffer.
> For the part of the buffer that gets highlighted (so the escape chars
> are not apparent) there is no problem.

Sounds like the likely culprit.  And the fix is still the same: move the
grep-highlighting-processing from font-lock to the process filter.


        Stefan

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Juri Linkov]

> M-x grep RET grep -nH -e "#" *.el */*.el RET
>
> The grep output shows all matches for `#'.
>
> C-x C-q to make the grep buffer writable.
>
> M-x flush-lines RET ###autoload RET
>
> Only the first few lines containing `###autoload' are flushed.

Your report is from 5 Apr 2011.  But this was already fixed 9 May 2011
by Yidong in http://debbugs.gnu.org/7952#103

Drew, could you verify that there is no bug anymore.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

> > M-x grep RET grep -nH -e "#" *.el */*.el RET
> >
> > The grep output shows all matches for `#'.
> >
> > C-x C-q to make the grep buffer writable.
> >
> > M-x flush-lines RET ###autoload RET
> >
> > Only the first few lines containing `###autoload' are flushed.
> 
> Your report is from 5 Apr 2011.  But this was already fixed 9 May 2011
> by Yidong in http://debbugs.gnu.org/7952#103
> 
> Drew, could you verify that there is no bug anymore.

Interesting.  Bug #8429 was not closed.

And yes, I tested it today before I sent that mail, starting from emacs -Q with
this build:

In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2013-01-30 on ODIEONE
Bzr revision: 111631 michael.albinus@gmx.de-20130130192046-nx4rskw7jemmtrw8
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -IC:/Devel/emacs/build/include --ldflags -LC:/Devel/emacs/build/lib'

I followed the recipe in the bug report.  At first I thought it was fixed, but
then I saw that there were still plenty of occurrences of the regexp matches
farther down in the buffer.  Try it and see what you find.

I used Emacs on Windows with grep from Cygwin, but that should not make any
difference.

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Juri Linkov]

> I followed the recipe in the bug report.  At first I thought it was fixed, but
> then I saw that there were still plenty of occurrences of the regexp matches
> farther down in the buffer.

When the grep process is not yet finished then of course flush-lines
can't remove all matches.  A running grep process adds more occurrences
that you have to remove with flush-lines when grep finishes.

> Try it and see what you find.

I tried and see that the bug is fixed, there are no remaining occurrences
when flush-lines is called after grep is finished.

If you still see the bug, please send the lines containing the
unremoved matches.  Do they contain escape sequences?

bug#8429: 24.0.50; regression: `flush-lines' does not flush all it should

[Drew Adams]

> > I followed the recipe in the bug report.  At first I 
> > thought it was fixed, but then I saw that there were
> > still plenty of occurrences of the regexp matches
> > farther down in the buffer.
> 
> When the grep process is not yet finished then of course flush-lines
> can't remove all matches.  A running grep process adds more 
> occurrences that you have to remove with flush-lines when grep
> finishes.

I thought I waited until grep was finished (final message plus mode-line
indication) - I specifically intended to as part of the test.  But apparently I
did not.  Trying again, I see that it is indeed fixed.

Very sorry for the noise.  I will close the bug.  Thx.