GNU bug report logs - #29360
26.0; Add full-buffer choice for `isearch-lazy-highlight'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29360 in the body.
You can then email your comments to 29360 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0; Add full-buffer choice for `isearch-lazy-highlight'
Date: Sun, 19 Nov 2017 11:05:54 -0800 (PST)

See bug #21092 for the motivation and reasons behind this bug report.
The aim of that bug report was never taken care of, which was to let
you force lazy-highlight highlighting on the full visible portion of
the buffer (where that refers to the full buffer or the current
buffer restriction).  IOW, optionally do not restrict highlighting
to what is currently on screen, as is done now.

The #21092 bug report mistakenly took a `nil' value of
`lazy-highlight-max-at-a-time' as meaning just that - just what the doc
said: act on the full buffer, not just on the text that is currently in
the window.  The true meaning of nil for that variable is just to
highlight all of the search hits visible in the window.

This new bug aims to finally get what was really being requested in
#21092: the ability to cause lazy highlight to act on the full buffer,
not just the text currently visible in the window.

The outcome of #21092 and other bugs led to the creation of variable
`isearch-lazy-highlight'.  At the end of #21092, realizing that closing
#21092 did not, in fact, respond to the aim of #21092, Juri suggested
that a separate report be filed, to request a new possible value for
`isearch-lazy-highlight' which will cause the full buffer to get the
lazy-highlight highlighting.  That is the purpose of this bug report:
please add such a `buffer' or `full-buffer' value.

In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32)
 of 2017-10-13
Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

Message #8 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Mon, 20 Nov 2017 22:53:55 +0200

> See bug #21092 for the motivation and reasons behind this bug report.
> The aim of that bug report was never taken care of, which was to let
> you force lazy-highlight highlighting on the full visible portion of
> the buffer (where that refers to the full buffer or the current
> buffer restriction).  IOW, optionally do not restrict highlighting
> to what is currently on screen, as is done now.
>
> The #21092 bug report mistakenly took a `nil' value of
> `lazy-highlight-max-at-a-time' as meaning just that - just what the doc
> said: act on the full buffer, not just on the text that is currently in
> the window.  The true meaning of nil for that variable is just to
> highlight all of the search hits visible in the window.
>
> This new bug aims to finally get what was really being requested in
> #21092: the ability to cause lazy highlight to act on the full buffer,
> not just the text currently visible in the window.
>
> The outcome of #21092 and other bugs led to the creation of variable
> `isearch-lazy-highlight'.  At the end of #21092, realizing that closing
> #21092 did not, in fact, respond to the aim of #21092, Juri suggested
> that a separate report be filed, to request a new possible value for
> `isearch-lazy-highlight' which will cause the full buffer to get the
> lazy-highlight highlighting.  That is the purpose of this bug report:
> please add such a `buffer' or `full-buffer' value.

Are you sure this should be part of isearch, not hi-lock?
Isearch used to highlight only the visible part of the buffer
because it is modal and doesn't allow the user to scroll the
current search match out of view.  Whereas hi-lock is intended
to highlight all matches in the whole buffer.

Message #11 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Mon, 20 Nov 2017 14:40:52 -0800 (PST)

> > See bug #21092 for the motivation and reasons behind this bug report.
> > The aim of that bug report was never taken care of, which was to let
> > you force lazy-highlight highlighting on the full visible portion of
> > the buffer (where that refers to the full buffer or the current
> > buffer restriction).  IOW, optionally do not restrict highlighting
> > to what is currently on screen, as is done now.
> >
> > The #21092 bug report mistakenly took a `nil' value of
> > `lazy-highlight-max-at-a-time' as meaning just that - just what the doc
> > said: act on the full buffer, not just on the text that is currently in
> > the window.  The true meaning of nil for that variable is just to
> > highlight all of the search hits visible in the window.
> >
> > This new bug aims to finally get what was really being requested in
> > #21092: the ability to cause lazy highlight to act on the full buffer,
> > not just the text currently visible in the window.
> >
> > The outcome of #21092 and other bugs led to the creation of variable
> > `isearch-lazy-highlight'.  At the end of #21092, realizing that closing
> > #21092 did not, in fact, respond to the aim of #21092, Juri suggested
> > that a separate report be filed, to request a new possible value for
> > `isearch-lazy-highlight' which will cause the full buffer to get the
> > lazy-highlight highlighting.  That is the purpose of this bug report:
> > please add such a `buffer' or `full-buffer' value.
> 
> Are you sure this should be part of isearch, not hi-lock?

Yes.

> Isearch used to highlight only the visible part of the buffer

Isearch also used to highlight only the current search hit -
no lazy-highlighting at all.  So what?  Did someone back
then argue that we shouldn't add lazy highlighting because
Isearch is only about finding the next search hit?  No.
Highlighting all visible hits is an improvement over the
pre-Emacs 22 behavior.  Being able to optionally highlight
all hits (visible in the window or not) adds other advantages.

> because it is modal and doesn't allow the user to scroll the
> current search match out of view. Whereas hi-lock is intended
> to highlight all matches in the whole buffer.

Just because Isearch has not previously had an option to
highlight all matches is not a reason it should not offer
such an option.  This is only about _optional_ behavior; it
proposes no change to the default lazy-highlighting behavior.

No relation to hi-lock. This is not about font-lock
highlighting.  It is about the "lazy-highlighting" 
of Isearch, during Isearch, for Isearch, being extended
to cover the whole buffer.  It is about incremental
search: being able to change the set of matches incrementally.

You can (now) set `lazy-highlight-cleanup' to nil (you can
even toggle it during Isearch) to keep the lazy-highlighting
when search is done.  That can be very useful.

And doing likewise without needing to totally exit Isearch
can also be useful, e.g., to search only _within_ the lazy
highlights - i.e., progressing search narrowing.  E.g,
combine multiple simple regexps (ANDing them) vs trying to
come up with a single, complex regexp to do the same thing.
Or being able to flip to searching the complement of the
lazy highlights.

Any such features expect, to be really useful, that the
lazy-highlighted zones include all of the search hits,
over the entire search space.  They are about a new
search that is related to the previous search.  They
should not be limited to whatever hits were visible in
the current window for the previous search.

Just because this is not the classic Isearch use case is
not a reason it isn't usefully added to Isearch.  It is
only about isearching - isearching zones that have been
found by isearching.  It's about isearching isearches...

Any argument about not being able to see highlighting past
the window is irrelevant - this is not about scrolling.

The motivation was already discussed in bug #21092.  That
bug should have taken care of this, but I expressed the
need in terms of `lazy-highlight-max-at-a-time', having
misunderstood that variable's purpose because its doc
string described, for a nil value, just what this bug
(#29360) asks for.

Message #14 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Thu, 23 Nov 2017 23:49:46 +0200

>> Are you sure this should be part of isearch, not hi-lock?
>
> No relation to hi-lock. This is not about font-lock
> highlighting.

hi-lock is not about font-lock highlighting too
when font-lock is not active.  See the variable
‘hi-lock-highlight-range’ with its default limit of 200000.

I'm thinking about using lazy-highlight in hi-lock
to overcome this limitation after adding support
for full-buffer to lazy-highlight.  Maybe better
to generalize the current implementation of
isearch-lazy-highlight-update to apply a function
given as an argument to perform different actions
on the found matches (by default, adding the lazy-highlight
overlay, and for hi-lock adding the hi-lock-overlay).

Message #17 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Thu, 23 Nov 2017 15:53:25 -0800 (PST)

> >> Are you sure this should be part of isearch, not hi-lock?

Yes, as I said.  And I gave several reasons.

> hi-lock is not about font-lock highlighting too
> when font-lock is not active.  See the variable
> ‘hi-lock-highlight-range’ with its default limit of 200000.
> 
> I'm thinking about using lazy-highlight in hi-lock
> to overcome this limitation after adding support
> for full-buffer to lazy-highlight.  Maybe better
> to generalize the current implementation of
> isearch-lazy-highlight-update to apply a function
> given as an argument to perform different actions
> on the found matches (by default, adding the lazy-highlight
> overlay, and for hi-lock adding the hi-lock-overlay).

A hi-lock feature doesn't correspond at all to what I
requested for this enhancement.  I specifically want
this as a feature of Isearch, for the reasons I gave.

I guess I'll need to do this for only my own code.
It's easy enough to do.  I was hoping not to have to
maintain yet another difference from vanilla Isearch,
and to give vanilla Isearch the same potential benefits.

Sorry I mentioned it now.  After you do what you will
do it will increase my maintenance burden rather than
lessening it, and it won't move vanilla Emacs in the
same direction.

I already provide users a way to act on the current
search hit in arbitrary ways.  The default action is
replacement - the default replacement is "" (delete).

Your way of providing something similar will no doubt
not offer exactly the same thing, and will anyway not
use the same approach of implementation.

No good deed goes unpunished, I guess.  I suppose I
should be happy to have inspired an improvement in
Emacs, even if it's not the improvement I requested.

Message #20 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Wed, 17 Oct 2018 22:47:26 -0700 (PDT)

Hi Juri,

It would really be good if this enhancement were made - for the
reasons I gave in bug #21092, and for other reasons. You asked me
(in bug #21092) to file this bug if I thought we needed such a
full-buffer possibility. This enhancement request was the result.

Did you not have a patch that took care of this? IIRC you then found
a problem with it wrt `follow-mode', but I thought you had a solution
for that too. (I thought the latter solution was provided by Artur's
`all-windows' value for `isearch-lazy-highlight' etc., which was added.)

There really is no comparison with hi-lock. Only Isearch provides
incremental matching (with highlighting) for a regexp etc. Being
able to try a search pattern, see the result as you type, adjust it,
etc. is very handy.

Then being able to leave the highlighting turned on (via nil
`lazy-highlight-cleanup') means you can do all kinds of things with
the resulting highlighted text. I do this now, but lack the ability to
force highlighting the full buffer. That was my aim in bug #21092.

What's the status of this feature? Can we add it to Emacs now?
I hope so.

Message #23 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Fri, 19 Oct 2018 01:18:21 +0300

> It would really be good if this enhancement were made - for the
> reasons I gave in bug #21092, and for other reasons. You asked me
> (in bug #21092) to file this bug if I thought we needed such a
> full-buffer possibility. This enhancement request was the result.

Actually, a full-buffer lazy-highlighting possibility already exists:

  (setq lazy-highlight-cleanup nil)
  (add-hook 'isearch-mode-end-hook
            (lambda ()
              (setq window-group-start-function (lambda (_w) (point-min)))
              (setq window-group-end-function (lambda (_w _u) (point-max)))))

But I agree that more straightforward customization would be better
with a clear value of the customizable variable.

> Did you not have a patch that took care of this? IIRC you then found
> a problem with it wrt `follow-mode', but I thought you had a solution
> for that too. (I thought the latter solution was provided by Artur's
> `all-windows' value for `isearch-lazy-highlight' etc., which was added.)

That patch was installed more than a year ago.

> What's the status of this feature? Can we add it to Emacs now?
> I hope so.

The reason why it's not yet finished is because it was unclear how to
integrate it with another similar feature of matches-counting (that counts
the number of matches in the full buffer).  The reasoning is the following:
both features require using the same loop in isearch-lazy-highlight
extending it to operate on the full buffer.  I know you will argue that
these are unrelated features and should be treated separately.
But implementation-wise they have only one difference:

1. buffer-matches-highlighting visits all matches and highlights them;

2. buffer-matches-counting visits all matches but doesn't highlight them
   (only counts)

Both need special treatment for possible slowdown in a
large buffer, so for performance reasons we need to add
a new customizable variable like lazy-buffer-max-at-a-time,
separate not to conflict with lazy-highlight-max-at-a-time.
The latter applies to the matches on the screen, the former
to the matches in the full buffer.

Message #26 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Thu, 18 Oct 2018 16:25:03 -0700 (PDT)

> > It would really be good if this enhancement were made - for the
> > reasons I gave in bug #21092, and for other reasons. You asked me
> > (in bug #21092) to file this bug if I thought we needed such a
> > full-buffer possibility. This enhancement request was the result.
> 
> Actually, a full-buffer lazy-highlighting possibility already exists:
> 
>   (setq lazy-highlight-cleanup nil)
>   (add-hook 'isearch-mode-end-hook
>             (lambda ()
>               (setq window-group-start-function (lambda (_w) (point-min)))
>               (setq window-group-end-function (lambda (_w _u) (point-max)))))

Thanks for this info. I don't really understand it, though. `C-h v window-group-start-function' tells me nothing, for instance - no doc, apparently.

(FWIW, I already use `isearch-mode-end-hook' quite a bit. Not that another
addition would necessarily break the bank. ;-))

> But I agree that more straightforward customization would be better
> with a clear value of the customizable variable.

Thank you. Will you please implement that when you get a chance?

> > Did you not have a patch that took care of this? IIRC you then found
> > a problem with it wrt `follow-mode', but I thought you had a solution
> > for that too. (I thought the latter solution was provided by Artur's
> > `all-windows' value for `isearch-lazy-highlight' etc., which was added.)
> 
> That patch was installed more than a year ago.

What is that patch? Is this about `window-group-start|end-function' or
is there some other way to force lazy highlighting to be done throughout
the buffer? If this is patch is now reflected in isearch.el, where would
I see it there?

> > What's the status of this feature? Can we add it to Emacs now?
> > I hope so.
> 
> The reason why it's not yet finished is because it was unclear how to
> integrate it with another similar feature of matches-counting (that counts
> the number of matches in the full buffer).  The reasoning is the following:
> both features require using the same loop in isearch-lazy-highlight
> extending it to operate on the full buffer.  I know you will argue that
> these are unrelated features and should be treated separately.

I won't argue any such thing, as I know nothing about this stuff. ;-)

> But implementation-wise they have only one difference:
> 
> 1. buffer-matches-highlighting visits all matches and highlights them;
> 2. buffer-matches-counting visits all matches but doesn't highlight them
>    (only counts)

That doesn't sound like a big difference, a priori. Can't you just
have a variable or a function argument that says whether to
highlight?

> Both need special treatment for possible slowdown in a
> large buffer, so for performance reasons we need to add
> a new customizable variable like lazy-buffer-max-at-a-time,
> separate not to conflict with lazy-highlight-max-at-a-time.
> The latter applies to the matches on the screen, the former
> to the matches in the full buffer.

I see. Sounds like that would be do-able, but I don't know anything
about the details. Hope you can/will resolve this sometime soon.

Thanks for taking a look.

Message #29 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Sat, 20 Oct 2018 10:10:35 -0700 (PDT)

Also, you said:

> But I agree that more straightforward customization would be
> better with a clear value of the customizable variable.

I agree about the usefulness of having a user option for this. But
here is one more consideration about that.

Personally, I have no problem in general with commands that bind
user options for their duration (and even with commands that set
user option values), as long as their doc strings tell users that they
do this.

But I think it is vanilla-Emacs practice to forbid such programmatic
changes of option values (even just during a command).

If so, then please also allow also for programmatic use. Code
should be able to bind a variable (non-option, if binding an
option is verboten) to control whether lazy highlighting is
full-buffer.

That is, I believe that Isearch has several cases where there are
both a user option and a non-option variable to control some
behavior. That should be the case for full-buffer highlighting too.

Message #32 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Sun, 21 Oct 2018 01:12:49 +0300

>> Actually, a full-buffer lazy-highlighting possibility already exists:
>>
>>   (setq lazy-highlight-cleanup nil)
>>   (add-hook 'isearch-mode-end-hook
>>             (lambda ()
>>               (setq window-group-start-function (lambda (_w) (point-min)))
>>               (setq window-group-end-function (lambda (_w _u) (point-max)))))
>
> Thanks for this info. I don't really understand it, though. `C-h
> v window-group-start-function' tells me nothing, for instance - no
> doc, apparently.
>
> (FWIW, I already use `isearch-mode-end-hook' quite a bit. Not that another
> addition would necessarily break the bank. ;-))

Sorry, I meant isearch-mode-hook, not isearch-mode-end-hook.
But this detail is not important for properly implementing
this feature.

>> That patch was installed more than a year ago.
>
> What is that patch? Is this about `window-group-start|end-function' or
> is there some other way to force lazy highlighting to be done throughout
> the buffer? If this is patch is now reflected in isearch.el, where would
> I see it there?

It's here: http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c5e66afa88d6ff8ad5c42318d85188ed477e7db2

>> Both need special treatment for possible slowdown in a
>> large buffer, so for performance reasons we need to add
>> a new customizable variable like lazy-buffer-max-at-a-time,
>> separate not to conflict with lazy-highlight-max-at-a-time.
>> The latter applies to the matches on the screen, the former
>> to the matches in the full buffer.
>
> I see. Sounds like that would be do-able, but I don't know anything
> about the details. Hope you can/will resolve this sometime soon.

The biggest obstacle is this - currently the traversal order of
visiting matches to lazy-highlight is the following:

1. start from the current match forward to the bottom of the window;

2. wrap from the bottom to the top of the window;

3. start from the top of the window down to the current match.

Visually you can observe how the current algorithm works by setting:

  (setq lazy-highlight-max-at-a-time 1
        lazy-highlight-initial-delay 1
        lazy-highlight-interval 1)

This works well when matches are highlighted only on the screen.

But when boundaries will be extended from the window to the full buffer,
the problem of performance creeps in.  Lazy-highlighting will work
in the following order:

1. start from the current match forward to the end of the buffer;

2. wrap from the end to the beginning of the buffer;

3. start from the beginning of the buffer down to the current match.

The problem is that matches in the upper part of the window might be
highlighted too late - only when all matches in the full buffer
are highlighted, and most of these matches likely will be outside
of the displayed part of the buffer in the window.

IOW, highlighting of the matches above the current match will be delayed
until all other matches in the whole buffer will get a highlighting
overlay.

Do you think we should change the algorithm and adapt it to highlighting
of the buffer?  Maybe we should first highlight matches on the bottom
and the top part of the window before going to highlight matches in
other parts of the buffer that are not visible on the screen?

Message #35 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Sat, 20 Oct 2018 16:09:29 -0700 (PDT)

> The biggest obstacle is this - currently the traversal order of
> visiting matches to lazy-highlight is the following:
> 
> 1. start from the current match forward to the bottom of the window;
> 
> 2. wrap from the bottom to the top of the window;
> 
> 3. start from the top of the window down to the current match.
> 
> Visually you can observe how the current algorithm works by setting:
> 
>   (setq lazy-highlight-max-at-a-time 1
>         lazy-highlight-initial-delay 1
>         lazy-highlight-interval 1)
> 
> This works well when matches are highlighted only on the screen.
> 
> But when boundaries will be extended from the window to the full buffer,
> the problem of performance creeps in.  Lazy-highlighting will work
> in the following order:
> 
> 1. start from the current match forward to the end of the buffer;
> 
> 2. wrap from the end to the beginning of the buffer;
> 
> 3. start from the beginning of the buffer down to the current match.
> 
> The problem is that matches in the upper part of the window might be
> highlighted too late - only when all matches in the full buffer
> are highlighted, and most of these matches likely will be outside
> of the displayed part of the buffer in the window.
> 
> IOW, highlighting of the matches above the current match will be delayed
> until all other matches in the whole buffer will get a highlighting
> overlay.
> 
> Do you think we should change the algorithm and adapt it to highlighting
> of the buffer?  Maybe we should first highlight matches on the bottom
> and the top part of the window before going to highlight matches in
> other parts of the buffer that are not visible on the screen?

Thanks for the explanation - very clear. I don't have any brilliant
insight into this.

I'd imagine that most uses of full-buffer highlighting are either:

1. On relatively small buffers, just for convenience of getting it
all done at once (i.e., non-lazy).

2. On any size buffer, for some other purpose than just Isearch.
IOW, use Isearch as a UI to get parts of a buffer highlighted (and
so propertized), for other purposes than just Isearch.

I'm not sure that #1 is a real use case. If it is then it's anyway
not problematic for the problem you mention - by definition.

It may be that for many of the #2 use cases immediate response
for the Isearch part is not that important. (Dunno.)

In any case, yes, your suggestion of first doing what we do now
(highlight the immediate area, using the current algorith), and
then following that with highlighting the rest of the buffer,
could be a good idea. Dunno how much that might change
the existing code.

Another possibility (?) is that for some of the #2 use cases it
might even be enough to highlight the rest of the buffer when
Emacs is idle. Again, dunno whether that option would be
important. But if we do that it should be controllable by
users and program, I'd think.

You likely have better ideas about all this. These are just a
few thoughts that came to mind now.

(BTW, I already sometimes see Isearch paint the lazy-highlighting
slowly, eventually coming down from the top of the window. Not
sure what the circumstances are in which I see that sometimes.
Probably when matching takes longer for some reason.)

Message #38 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Sun, 21 Oct 2018 22:06:48 +0300

[Message part 1 (text/plain, inline)]

> In any case, yes, your suggestion of first doing what we do now
> (highlight the immediate area, using the current algorith), and
> then following that with highlighting the rest of the buffer,
> could be a good idea. Dunno how much that might change
> the existing code.

In the patch attached below, a new function isearch-lazy-highlight-buffer-update
is a copy of isearch-lazy-highlight-update with some changes specific
to highlighting the full buffer.  It seems making a duplicate function
is necessary because adding more full-buffer specific conditions to
the existing function isearch-lazy-highlight-update will make it
unmaintainable.

Only a part of the function (that highlights the match) is extracted
into a new function isearch-lazy-highlight-match and shared among these
aforementioned two functions.

[isearch-lazy-highlight-buffer.1.patch (text/x-diff, inline)]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 1e785a44c5..cb9e72526b 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -304,7 +304,7 @@ isearch-fail
 
 (defcustom isearch-lazy-highlight t
   "Controls the lazy-highlighting during incremental search.
-When non-nil, all text in the buffer matching the current search
+When non-nil, all text on the screen matching the current search
 string is highlighted lazily (see `lazy-highlight-initial-delay'
 and `lazy-highlight-interval').
 
@@ -316,6 +316,15 @@ isearch-lazy-highlight
   :group 'lazy-highlight
   :group 'isearch)
 
+(defcustom isearch-lazy-highlight-buffer nil
+  "Controls the lazy-highlighting of the whole buffer.
+When non-nil, all text in the buffer matching the current search
+string is highlighted lazily (see `lazy-highlight-initial-delay',
+`lazy-highlight-interval' and `lazy-highlight-buffer-max-at-a-time')."
+  :type 'boolean
+  :group 'lazy-highlight
+  :group 'isearch)
+
 ;;; Lazy highlight customization.
 
 (defgroup lazy-highlight nil
@@ -351,6 +360,15 @@ lazy-highlight-max-at-a-time
 		 (integer :tag "Some"))
   :group 'lazy-highlight)
 
+(defcustom lazy-highlight-buffer-max-at-a-time 20
+  "Maximum matches to highlight at a time (for `isearch-lazy-highlight-buffer').
+Larger values may reduce Isearch's responsiveness to user input;
+smaller values make matches highlight slowly.
+A value of nil means highlight all matches shown in the buffer."
+  :type '(choice (const :tag "All" nil)
+		 (integer :tag "Some"))
+  :group 'lazy-highlight)
+
 (defface lazy-highlight
   '((((class color) (min-colors 88) (background light))
      (:background "paleturquoise"))
@@ -3290,13 +3308,13 @@ isearch-lazy-highlight-search
 				(+ isearch-lazy-highlight-start
 				   ;; Extend bound to match whole string at point
 				   (1- (length isearch-lazy-highlight-last-string)))
-			      (window-group-end)))
+			      (if isearch-lazy-highlight-buffer (point-max) (window-group-end))))
 		     (max (or isearch-lazy-highlight-start-limit (point-min))
 			  (if isearch-lazy-highlight-wrapped
 			      (- isearch-lazy-highlight-end
 				 ;; Extend bound to match whole string at point
 				 (1- (length isearch-lazy-highlight-last-string)))
-			    (window-group-start))))))
+			    (if isearch-lazy-highlight-buffer (point-min) (window-group-start)))))))
 	;; Use a loop like in `isearch-search'.
 	(while retry
 	  (setq success (isearch-search-string
@@ -3317,6 +3335,20 @@ isearch-lazy-highlight-start
   (lazy-highlight-cleanup t) ;remove old overlays
   (isearch-lazy-highlight-update))
 
+(defvar isearch-lazy-highlight-match-function #'isearch-lazy-highlight-match
+  "Function that highlights the found match.
+The function accepts two arguments: the beginning and the end of the match.")
+
+(defun isearch-lazy-highlight-match (mb me)
+  (let ((ov (make-overlay mb me)))
+    (push ov isearch-lazy-highlight-overlays)
+    ;; 1000 is higher than ediff's 100+,
+    ;; but lower than isearch main overlay's 1001
+    (overlay-put ov 'priority 1000)
+    (overlay-put ov 'face 'lazy-highlight)
+    (unless (eq isearch-lazy-highlight 'all-windows)
+      (overlay-put ov 'window (selected-window)))))
+
 (defun isearch-lazy-highlight-update ()
   "Update highlighting of other matches for current search."
   (let ((max lazy-highlight-max-at-a-time)
@@ -3353,16 +3385,8 @@ isearch-lazy-highlight-update
 					(window-group-start)))
 				(setq found nil)
 			      (forward-char -1)))
-
 			;; non-zero-length match
-			(let ((ov (make-overlay mb me)))
-			  (push ov isearch-lazy-highlight-overlays)
-			  ;; 1000 is higher than ediff's 100+,
-			  ;; but lower than isearch main overlay's 1001
-			  (overlay-put ov 'priority 1000)
-			  (overlay-put ov 'face 'lazy-highlight)
-			  (unless (eq isearch-lazy-highlight 'all-windows)
-                            (overlay-put ov 'window (selected-window)))))
+			(funcall isearch-lazy-highlight-match-function mb me))
 		      ;; Remember the current position of point for
 		      ;; the next call of `isearch-lazy-highlight-update'
 		      ;; when `lazy-highlight-max-at-a-time' is too small.
@@ -3384,11 +3408,75 @@ isearch-lazy-highlight-update
 			(setq isearch-lazy-highlight-start (window-group-end))
 			(goto-char (min (or isearch-lazy-highlight-end-limit (point-max))
 					(window-group-end))))))))
-	    (unless nomore
+	    (if nomore
+                (when isearch-lazy-highlight-buffer
+                  (setq isearch-lazy-highlight-start (window-group-start))
+                  (setq isearch-lazy-highlight-end (window-group-end))
+                  (setq isearch-lazy-highlight-wrapped nil)
+                  (run-at-time lazy-highlight-interval nil
+			       'isearch-lazy-highlight-buffer-update))
 	      (setq isearch-lazy-highlight-timer
 		    (run-at-time lazy-highlight-interval nil
 				 'isearch-lazy-highlight-update)))))))))
 
+(defun isearch-lazy-highlight-buffer-update ()
+  "Update highlighting of other matches in the whole buffer."
+  (let ((max lazy-highlight-buffer-max-at-a-time)
+        (looping t)
+        nomore)
+    (with-local-quit
+      (save-excursion
+	(save-match-data
+	  (goto-char (if isearch-lazy-highlight-forward
+			 isearch-lazy-highlight-end
+		       isearch-lazy-highlight-start))
+	  (while looping
+	    (let ((found (isearch-lazy-highlight-search)))
+	      (when max
+		(setq max (1- max))
+		(if (<= max 0)
+		    (setq looping nil)))
+	      (if found
+		  (let ((mb (match-beginning 0))
+			(me (match-end 0)))
+		    (if (= mb me)	;zero-length match
+			(if isearch-lazy-highlight-forward
+			    (if (= mb (if isearch-lazy-highlight-wrapped
+					  (window-group-start)
+				        (point-max)))
+				(setq found nil)
+			      (forward-char 1))
+			  (if (= mb (if isearch-lazy-highlight-wrapped
+					(window-group-end)
+				      (point-min)))
+			      (setq found nil)
+			    (forward-char -1)))
+		      ;; non-zero-length match
+		      (funcall isearch-lazy-highlight-match-function mb me))
+		    ;; Remember the current position of point for
+		    ;; the next call of `isearch-lazy-highlight-update'
+		    ;; when `lazy-highlight-max-at-a-time' is too small.
+		    (if isearch-lazy-highlight-forward
+			(setq isearch-lazy-highlight-end (point))
+		      (setq isearch-lazy-highlight-start (point)))))
+	      ;; not found or zero-length match at the search bound
+	      (if (not found)
+		  (if isearch-lazy-highlight-wrapped
+		      (setq looping nil
+			    nomore  t)
+		    (setq isearch-lazy-highlight-wrapped t)
+		    (if isearch-lazy-highlight-forward
+			(progn
+			  (setq isearch-lazy-highlight-end (point-min))
+			  (goto-char (point-min)))
+		      (setq isearch-lazy-highlight-start (point-max))
+		      (goto-char (point-max)))))))
+	  (if nomore
+              (message "Finished lazy-highlighting the buffer")
+	    (setq isearch-lazy-highlight-timer
+		  (run-at-time lazy-highlight-interval nil
+			       'isearch-lazy-highlight-buffer-update))))))))
+
 (defun isearch-resume (string regexp word forward message case-fold)
   "Resume an incremental search.
 STRING is the string or regexp searched for.

Message #41 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Sun, 21 Oct 2018 20:33:35 -0700 (PDT)

> > In any case, yes, your suggestion of first doing what we do now
> > (highlight the immediate area, using the current algorith), and
> > then following that with highlighting the rest of the buffer,
> > could be a good idea. Dunno how much that might change
> > the existing code.
> 
> In the patch attached below, a new function isearch-lazy-highlight-buffer-
> update
> is a copy of isearch-lazy-highlight-update with some changes specific
> to highlighting the full buffer.  It seems making a duplicate function
> is necessary because adding more full-buffer specific conditions to
> the existing function isearch-lazy-highlight-update will make it
> unmaintainable.
> 
> Only a part of the function (that highlights the match) is extracted
> into a new function isearch-lazy-highlight-match and shared among these
> aforementioned two functions.

Thanks for doing this.  A cursory check tells me it works.

One main comment and a couple of minor ones.

1. The main one is to remind you of what I said earlier:

> [P]lease also allow also for programmatic use. Code
> should be able to bind a variable (non-option, if binding an
> option is verboten) to control whether lazy highlighting is
> full-buffer.
>
> That is, I believe that Isearch has several cases where there
> are both a user option and a non-option variable to control some
> behavior. That should be the case for full-buffer highlighting too.

As also stated earlier, I expect that the main use cases
will involve using "Isearch as a UI to get parts of a buffer
highlighted (and so propertized), for other purposes than
just Isearch."

Would you please consider adding such non-option variable
control? I don't need it for my own code, because I don't
hesitate to offer commands that bind user options.  But
vanilla Emacs feels differently.


2. Please think about changing "on the screen" (it's in a
couple places) to something like "currently visible in the
window". (Or "currently visible on the screen" if you want
this to apply to multiple windows.)

This is actually something that's bothered me before, but
it becomes more important now that we've introduced the
possibility of highlighting "beyond the screen", so to speak.

My concern here is that "on the screen" can be misinterpreted
as more than just the text that is currently shown.  Even
talking about the text that is "in" a window is problematic
for someone who hasn't gotten acquainted with Emacs jargon.

A user can mistakenly think that all of a buffer's text is
"in" a window that shows the buffer, even the parts that are
not currently shown there.  We should try to somehow get
across the fact that only text in parts of the buffer that
you show by scrolling to it gets highlighted.

Yes, there could be some confusion regarding invisible text
if we say something like currently "visible" or "shown".
But I don't think "on the screen" is clear enough.  It's not
obvious to a user that there is some text in the buffer that
is not "on the screen".

Maybe it would be best to explicitly state what's involved,
at each place where we speak of "screen" - including perhaps
`isearch-allow-scroll'.

Perhaps say that unless `isearch-lazy-highlight-buffer'
is non-nil lazy highlighting highlights only the parts of
the buffer you can currently see (but that other parts
already highlighted remain highlighted).

Also, as a casual user might well wonder why we have
option `isearch-lazy-highlight-buffer', it might be good
to mention a use case: you might want to highlight all
occurrences and then process all of them programmatically.
(And mention option `lazy-highlight-cleanup', for instance.)

Dunno how important this is, as most users will neither
look at `isearch-lazy-highlight-buffer' nor imagine that
they might want to highlight text that they don't yet see.
But for a user who does, it could help to give them the
idea that they might do something with the highlighted
text.


3. Finally, I think that lazy-highlighting the full buffer
will typically go hand in hand with keeping the highlighting.
That is, non-nil `isearch-lazy-highlight-buffer' will be
used with nil `isearch-lazy-highlight-cleanup', and vice
versa.  It might be useful to make it easy to couple the two.

(In my own code, at least, I've added a key that toggles
`isearch-lazy-highlight-buffer' and sets
`isearch-lazy-highlight-cleanup' to `(not
isearch-lazy-highlight-buffer)'.  I know that Emacs won't
want keys that toggle option values, so I'm not sure what
Emacs would want to do to make it easy for a user to couple
the two.)


4. Another possible addition might be an indicator somewhere
(in the prompt? lighter?) to show that lazy highlighting
is in progress.  Dunno whether that's needed.  It's good
that you added the message letting you know when it's done.


Thanks again for implementing this feature.  I think you
can close this bug now.  I've played with it a bit, and
it seems to work fine.

 - Drew

Message #44 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Wed, 24 Oct 2018 01:05:17 +0300

> Thanks for doing this.  A cursory check tells me it works.

Bad news: when I tried to type an isearch string on a large file
(about 1MB) after typing the first character 'e' lazy-highlighting
became unresponsive busy highlighting all 50000 matches in that file.

Do you think we should start lazy-highlighting for the full buffer
only when the length of the search string is more than 1 character?

Message #47 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Tue, 23 Oct 2018 15:51:14 -0700 (PDT)

> Bad news: when I tried to type an isearch string on a large file
> (about 1MB) after typing the first character 'e' lazy-highlighting
> became unresponsive busy highlighting all 50000 matches in that file.
> 
> Do you think we should start lazy-highlighting for the full buffer
> only when the length of the search string is more than 1 character?


Absolutely.  Like with any similar interaction in
Emacs the delay before starting should be a user
option.  Users are different, and user machines etc.
are different.

Another possibility is for the option value to be
a choice of either:

* A non-negative number of seconds (or maybe natnump)
* A cons (SIZE . DELAY), where SIZE is the buffer-size 
  threshold: no delay if the buffer is no larger than
  this, and DELAY seconds delay if greater than this.

Another possibility, to accomplish the same thing (delay
and threshold) is to have two different options:

lazy-highlight-delay - just the delay
lazy-highlight-threshold - just the size threshold

The latter approach is what I use in Icicles, for
several such settings, e.g.
`icicle-incremental-completion-delay' and
`icicle-incremental-completion-threshold'

But maybe the former approach is simpler.

Message #50 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Thu, 25 Oct 2018 02:11:25 +0300

[Message part 1 (text/plain, inline)]

> Absolutely.  Like with any similar interaction in
> Emacs the delay before starting should be a user
> option.  Users are different, and user machines etc.
> are different.

You are right, with a small delay lazy-highlighting is responsive.

In the attached patch I also added a separate variable that you asked for
programmatic use, changed text to "currently visible on the screen" and
mentioned lazy-highlight-cleanup.

The patch is extensively tested for various scenarios:

[isearch-lazy-highlight-buffer.2.patch (text/x-diff, inline)]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 1e785a44c5..dae85208cb 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -304,9 +304,9 @@ isearch-fail
 
 (defcustom isearch-lazy-highlight t
   "Controls the lazy-highlighting during incremental search.
-When non-nil, all text in the buffer matching the current search
-string is highlighted lazily (see `lazy-highlight-initial-delay'
-and `lazy-highlight-interval').
+When non-nil, all text currently visible on the screen
+matching the current search string is highlighted lazily
+(see `lazy-highlight-initial-delay' and `lazy-highlight-interval').
 
 When multiple windows display the current buffer, the
 highlighting is displayed only on the selected window, unless
@@ -316,6 +316,17 @@ isearch-lazy-highlight
   :group 'lazy-highlight
   :group 'isearch)
 
+(defcustom isearch-lazy-highlight-buffer nil
+  "Controls the lazy-highlighting of the full buffer.
+When non-nil, all text in the buffer matching the current search
+string is highlighted lazily (see `lazy-highlight-initial-delay',
+`lazy-highlight-interval' and `lazy-highlight-buffer-max-at-a-time').
+This is useful when `lazy-highlight-cleanup' is customized to nil
+and doesn't remove full-buffer highlighting after a search."
+  :type 'boolean
+  :group 'lazy-highlight
+  :group 'isearch)
+
 ;;; Lazy highlight customization.
 
 (defgroup lazy-highlight nil
@@ -351,6 +362,15 @@ lazy-highlight-max-at-a-time
 		 (integer :tag "Some"))
   :group 'lazy-highlight)
 
+(defcustom lazy-highlight-buffer-max-at-a-time 20
+  "Maximum matches to highlight at a time (for `isearch-lazy-highlight-buffer').
+Larger values may reduce Isearch's responsiveness to user input;
+smaller values make matches highlight slowly.
+A value of nil means highlight all matches shown in the buffer."
+  :type '(choice (const :tag "All" nil)
+		 (integer :tag "Some"))
+  :group 'lazy-highlight)
+
 (defface lazy-highlight
   '((((class color) (min-colors 88) (background light))
      (:background "paleturquoise"))
@@ -3178,6 +3198,7 @@ isearch-lazy-highlight-window
 (defvar isearch-lazy-highlight-window-group nil)
 (defvar isearch-lazy-highlight-window-start nil)
 (defvar isearch-lazy-highlight-window-end nil)
+(defvar isearch-lazy-highlight-buffer-p nil)
 (defvar isearch-lazy-highlight-case-fold-search nil)
 (defvar isearch-lazy-highlight-regexp nil)
 (defvar isearch-lazy-highlight-lax-whitespace nil)
@@ -3226,10 +3247,12 @@ isearch-lazy-highlight-new-loop
 			  isearch-lax-whitespace))
 		 (not (eq isearch-lazy-highlight-regexp-lax-whitespace
 			  isearch-regexp-lax-whitespace))
-                 (not (= (window-group-start)
-                         isearch-lazy-highlight-window-start))
-                 (not (= (window-group-end) ; Window may have been split/joined.
-			 isearch-lazy-highlight-window-end))
+		 (not (or isearch-lazy-highlight-buffer-p
+			  (= (window-group-start)
+			     isearch-lazy-highlight-window-start)))
+		 (not (or isearch-lazy-highlight-buffer-p
+			  (= (window-group-end) ; Window may have been split/joined.
+			     isearch-lazy-highlight-window-end)))
 		 (not (eq isearch-forward
 			  isearch-lazy-highlight-forward))
 		 ;; In case we are recovering from an error.
@@ -3247,6 +3270,7 @@ isearch-lazy-highlight-new-loop
           isearch-lazy-highlight-window-group (selected-window-group)
 	  isearch-lazy-highlight-window-start (window-group-start)
 	  isearch-lazy-highlight-window-end   (window-group-end)
+	  isearch-lazy-highlight-buffer-p     isearch-lazy-highlight-buffer
 	  ;; Start lazy-highlighting at the beginning of the found
 	  ;; match (`isearch-other-end').  If no match, use point.
 	  ;; One of the next two variables (depending on search direction)
@@ -3264,12 +3288,22 @@ isearch-lazy-highlight-new-loop
 	  isearch-lazy-highlight-regexp-lax-whitespace isearch-regexp-lax-whitespace
 	  isearch-lazy-highlight-regexp-function  isearch-regexp-function
 	  isearch-lazy-highlight-forward      isearch-forward)
+    ;; Extend start/end to match whole string at point
+    (if isearch-lazy-highlight-forward
+        (setq isearch-lazy-highlight-start
+	      (min (+ isearch-lazy-highlight-start
+		      (1- (length isearch-lazy-highlight-last-string)))
+		   (point-max)))
+      (setq isearch-lazy-highlight-end
+	    (max (- isearch-lazy-highlight-end
+		    (1- (length isearch-lazy-highlight-last-string)))
+		 (point-min))))
     (unless (equal isearch-string "")
       (setq isearch-lazy-highlight-timer
             (run-with-idle-timer lazy-highlight-initial-delay nil
                                  'isearch-lazy-highlight-start)))))
 
-(defun isearch-lazy-highlight-search ()
+(defun isearch-lazy-highlight-search (string bound)
   "Search ahead for the next or previous match, for lazy highlighting.
 Attempt to do the search exactly the way the pending Isearch would."
   (condition-case nil
@@ -3283,24 +3317,10 @@ isearch-lazy-highlight-search
 	    (isearch-forward isearch-lazy-highlight-forward)
 	    (search-invisible nil)	; don't match invisible text
 	    (retry t)
-	    (success nil)
-	    (bound (if isearch-lazy-highlight-forward
-		       (min (or isearch-lazy-highlight-end-limit (point-max))
-			    (if isearch-lazy-highlight-wrapped
-				(+ isearch-lazy-highlight-start
-				   ;; Extend bound to match whole string at point
-				   (1- (length isearch-lazy-highlight-last-string)))
-			      (window-group-end)))
-		     (max (or isearch-lazy-highlight-start-limit (point-min))
-			  (if isearch-lazy-highlight-wrapped
-			      (- isearch-lazy-highlight-end
-				 ;; Extend bound to match whole string at point
-				 (1- (length isearch-lazy-highlight-last-string)))
-			    (window-group-start))))))
+	    (success nil))
 	;; Use a loop like in `isearch-search'.
 	(while retry
-	  (setq success (isearch-search-string
-			 isearch-lazy-highlight-last-string bound t))
+	  (setq success (isearch-search-string string bound t))
 	  ;; Clear RETRY unless the search predicate says
 	  ;; to skip this search hit.
 	  (if (or (not success)
@@ -3312,6 +3332,20 @@ isearch-lazy-highlight-search
 	success)
     (error nil)))
 
+(defvar isearch-lazy-highlight-match-function #'isearch-lazy-highlight-match
+  "Function that highlights the found match.
+The function accepts two arguments: the beginning and the end of the match.")
+
+(defun isearch-lazy-highlight-match (mb me)
+  (let ((ov (make-overlay mb me)))
+    (push ov isearch-lazy-highlight-overlays)
+    ;; 1000 is higher than ediff's 100+,
+    ;; but lower than isearch main overlay's 1001
+    (overlay-put ov 'priority 1000)
+    (overlay-put ov 'face 'lazy-highlight)
+    (unless (eq isearch-lazy-highlight 'all-windows)
+      (overlay-put ov 'window (selected-window)))))
+
 (defun isearch-lazy-highlight-start ()
   "Start a new lazy-highlight updating loop."
   (lazy-highlight-cleanup t) ;remove old overlays
@@ -3321,19 +3355,32 @@ isearch-lazy-highlight-update
   "Update highlighting of other matches for current search."
   (let ((max lazy-highlight-max-at-a-time)
         (looping t)
-        nomore)
+        nomore window-start window-end)
     (with-local-quit
       (save-selected-window
 	(if (and (window-live-p isearch-lazy-highlight-window)
 		 (not (memq (selected-window) isearch-lazy-highlight-window-group)))
 	    (select-window isearch-lazy-highlight-window))
+	(setq window-start (window-group-start))
+	(setq window-end (window-group-end))
 	(save-excursion
 	  (save-match-data
 	    (goto-char (if isearch-lazy-highlight-forward
 			   isearch-lazy-highlight-end
 			 isearch-lazy-highlight-start))
 	    (while looping
-	      (let ((found (isearch-lazy-highlight-search)))
+	      (let* ((bound (if isearch-lazy-highlight-forward
+		                (min (or isearch-lazy-highlight-end-limit (point-max))
+			             (if isearch-lazy-highlight-wrapped
+				         isearch-lazy-highlight-start
+			               window-end))
+		              (max (or isearch-lazy-highlight-start-limit (point-min))
+			           (if isearch-lazy-highlight-wrapped
+			               isearch-lazy-highlight-end
+			             window-start))))
+		     (found (isearch-lazy-highlight-search
+			     isearch-lazy-highlight-last-string
+			     bound)))
 		(when max
 		  (setq max (1- max))
 		  (if (<= max 0)
@@ -3345,24 +3392,16 @@ isearch-lazy-highlight-update
 			  (if isearch-lazy-highlight-forward
 			      (if (= mb (if isearch-lazy-highlight-wrapped
 					    isearch-lazy-highlight-start
-					  (window-group-end)))
+					  window-end))
 				  (setq found nil)
 				(forward-char 1))
 			    (if (= mb (if isearch-lazy-highlight-wrapped
 					  isearch-lazy-highlight-end
-					(window-group-start)))
+					window-start))
 				(setq found nil)
 			      (forward-char -1)))
-
 			;; non-zero-length match
-			(let ((ov (make-overlay mb me)))
-			  (push ov isearch-lazy-highlight-overlays)
-			  ;; 1000 is higher than ediff's 100+,
-			  ;; but lower than isearch main overlay's 1001
-			  (overlay-put ov 'priority 1000)
-			  (overlay-put ov 'face 'lazy-highlight)
-			  (unless (eq isearch-lazy-highlight 'all-windows)
-                            (overlay-put ov 'window (selected-window)))))
+			(funcall isearch-lazy-highlight-match-function mb me))
 		      ;; Remember the current position of point for
 		      ;; the next call of `isearch-lazy-highlight-update'
 		      ;; when `lazy-highlight-max-at-a-time' is too small.
@@ -3378,17 +3417,83 @@ isearch-lazy-highlight-update
 		      (setq isearch-lazy-highlight-wrapped t)
 		      (if isearch-lazy-highlight-forward
 			  (progn
-			    (setq isearch-lazy-highlight-end (window-group-start))
+			    (setq isearch-lazy-highlight-end window-start)
 			    (goto-char (max (or isearch-lazy-highlight-start-limit (point-min))
-					    (window-group-start))))
-			(setq isearch-lazy-highlight-start (window-group-end))
+					    window-start)))
+			(setq isearch-lazy-highlight-start window-end)
 			(goto-char (min (or isearch-lazy-highlight-end-limit (point-max))
-					(window-group-end))))))))
-	    (unless nomore
+					window-end)))))))
+	    (if nomore
+		(when isearch-lazy-highlight-buffer-p
+		  (if isearch-lazy-highlight-forward
+		      (setq isearch-lazy-highlight-end (point-min))
+		    (setq isearch-lazy-highlight-start (point-max)))
+		  (run-at-time lazy-highlight-interval nil
+			       'isearch-lazy-highlight-buffer-update))
 	      (setq isearch-lazy-highlight-timer
 		    (run-at-time lazy-highlight-interval nil
 				 'isearch-lazy-highlight-update)))))))))
 
+(defun isearch-lazy-highlight-buffer-update ()
+  "Update highlighting of other matches in the whole buffer."
+  (let ((max lazy-highlight-buffer-max-at-a-time)
+        (looping t)
+        nomore window-start window-end)
+    (with-local-quit
+      (save-selected-window
+	(if (and (window-live-p isearch-lazy-highlight-window)
+		 (not (memq (selected-window) isearch-lazy-highlight-window-group)))
+	    (select-window isearch-lazy-highlight-window))
+	(setq window-start (window-group-start))
+	(setq window-end (window-group-end))
+	(save-excursion
+	  (save-match-data
+	    (goto-char (if isearch-lazy-highlight-forward
+			   isearch-lazy-highlight-end
+			 isearch-lazy-highlight-start))
+	    (while looping
+	      (let* ((bound (if isearch-lazy-highlight-forward
+				(or isearch-lazy-highlight-end-limit (point-max))
+			      (or isearch-lazy-highlight-start-limit (point-min))))
+		     (found (isearch-lazy-highlight-search
+			     isearch-lazy-highlight-last-string
+			     bound)))
+		(when max
+		  (setq max (1- max))
+		  (if (<= max 0)
+		      (setq looping nil)))
+		(if found
+		    (let ((mb (match-beginning 0))
+			  (me (match-end 0)))
+		      (if (= mb me)	;zero-length match
+			  (if isearch-lazy-highlight-forward
+			      (if (= mb (point-max))
+				  (setq found nil)
+				(forward-char 1))
+			    (if (= mb (point-min))
+				(setq found nil)
+			      (forward-char -1)))
+			;; Already highlighted by isearch-lazy-highlight-update
+			(unless (or (and (>= mb window-start) (<= me window-end))
+                                    (not isearch-lazy-highlight-buffer-p))
+			  ;; non-zero-length match
+			  (funcall isearch-lazy-highlight-match-function mb me)))
+		      ;; Remember the current position of point for
+		      ;; the next call of `isearch-lazy-highlight-update'
+		      ;; when `lazy-highlight-buffer-max-at-a-time' is too small.
+		      (if isearch-lazy-highlight-forward
+			  (setq isearch-lazy-highlight-end (point))
+			(setq isearch-lazy-highlight-start (point)))))
+
+		;; not found or zero-length match at the search bound
+		(if (not found)
+		    (setq looping nil
+			  nomore  t))))
+	    (unless nomore
+	      (setq isearch-lazy-highlight-timer
+		    (run-at-time lazy-highlight-interval nil
+				 'isearch-lazy-highlight-buffer-update)))))))))
+
 (defun isearch-resume (string regexp word forward message case-fold)
   "Resume an incremental search.
 STRING is the string or regexp searched for.

Message #53 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: 26.0; Add full-buffer choice for
 `isearch-lazy-highlight'
Date: Wed, 24 Oct 2018 17:28:40 -0700 (PDT)

> > Absolutely.  Like with any similar interaction in
> > Emacs the delay before starting should be a user
> > option.  Users are different, and user machines etc.
> > are different.
> 
> You are right, with a small delay lazy-highlighting is responsive.
> 
> In the attached patch I also added a separate variable that you asked for
> programmatic use, changed text to "currently visible on the screen" and
> mentioned lazy-highlight-cleanup.
> 
> The patch is extensively tested for various scenarios:

Super. Thanks for working on this.

Message #56 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: 26.0;
 Add full-buffer choice for `isearch-lazy-highlight'
Date: Sat, 27 Oct 2018 23:28:18 +0300

tags 29360 fixed
close 29360 27.1
thanks

>> In the attached patch I also added a separate variable that you asked for
>> programmatic use, changed text to "currently visible on the screen" and
>> mentioned lazy-highlight-cleanup.
>>
>> The patch is extensively tested for various scenarios:
>
> Super. Thanks for working on this.

Installed in 3dd16a89bf.

Message #63 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29321: Isearch hit count
Date: Sat, 27 Oct 2018 17:35:03 -0700 (PDT)

Moving this back to bug #29360...

> > > Juri: Adapting your patch to my code, I notice that it uses
> > > `isearch-lazy-highlight-buffer' instead of
> > > `isearch-lazy-highlight-buffer-p' now.  Did you intend that
> > > change back to using the option in those occurrences?
> >
> > More exactly, it seems like you use a mix now of
> > `isearch-lazy-highlight-buffer' and `lazy-highlight-buffer'
> > (no `isearch-'), the latter of which is not in your previous
> > patches at all (for bug #29360).
> >
> > Please advise.  What replaced what exactly, in this regard?
> 
> I guess that since your patch to the bug thread you just renamed
> isearch-lazy-highlight-buffer to lazy-highlight-buffer and you
> renamed isearch-lazy-highlight-buffer-p to
> isearch-lazy-highlight-buffer.
> 
> If something other or more than that was involved, please let me know.

Hm.  Seems like you also removed `isearch-lazy-highlight-match-function',
which was present in your patches.  Why is that?  I know that you
you used only `isearch-lazy-highlight-match' as the value, but did
you have something else in mind for it?  Can I expect that you will
add back that variable?

Message #66 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 29360 <at> debbugs.gnu.org
Subject: Re: bug#29360: Add full-buffer choice for `isearch-lazy-highlight'
Date: Mon, 29 Oct 2018 00:34:32 +0200

>> I guess that since your patch to the bug thread you just renamed
>> isearch-lazy-highlight-buffer to lazy-highlight-buffer and you
>> renamed isearch-lazy-highlight-buffer-p to
>> isearch-lazy-highlight-buffer.

I renamed isearch-lazy-highlight-buffer to lazy-highlight-buffer
after realizing that the same feature might be needed also in
query-replace, therefore generalized it by removing the prefix
isearch-.

> Hm.  Seems like you also removed `isearch-lazy-highlight-match-function',
> which was present in your patches.  Why is that?  I know that you
> you used only `isearch-lazy-highlight-match' as the value, but did
> you have something else in mind for it?  Can I expect that you will
> add back that variable?

I strive to keep my changes as minimal as possible to not introduce
unused variables.  However, if you can demonstrate a real need
we could add it when necessary.

Message #69 received at 29360 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 29360 <at> debbugs.gnu.org
Subject: RE: bug#29360: Add full-buffer choice for `isearch-lazy-highlight'
Date: Sun, 28 Oct 2018 15:57:21 -0700 (PDT)

> >> I guess that since your patch to the bug thread you just renamed
> >> isearch-lazy-highlight-buffer to lazy-highlight-buffer and you
> >> renamed isearch-lazy-highlight-buffer-p to
> >> isearch-lazy-highlight-buffer.
> 
> I renamed isearch-lazy-highlight-buffer to lazy-highlight-buffer
> after realizing that the same feature might be needed also in
> query-replace, therefore generalized it by removing the prefix
> isearch-.

Yes, I figured that.

> > Hm.  Seems like you also removed `isearch-lazy-highlight-match-
> function',
> > which was present in your patches.  Why is that?  I know that you
> > you used only `isearch-lazy-highlight-match' as the value, but did
> > you have something else in mind for it?  Can I expect that you will
> > add back that variable?
> 
> I strive to keep my changes as minimal as possible to not introduce
> unused variables.  However, if you can demonstrate a real need
> we could add it when necessary.

I'm OK with all that you've done.  I may leave the variable
in my code, probably only out of laziness to remove it. ;-)

Thanks for your work on this.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.