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

[Drew Adams]

> > 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