bug#21092: 25.0.50; Option `lazy-highlight-max-at-a-time' does not work

[Drew Adams <drew.adams@oracle.com>]

> > Please see the simple patch I sent, which I think takes care of this.
> 
> If we are going to extend highlighting beyond the displayed portion of
> the buffer, then your proposed patch needs more work, IMO.  AFAIU,
> with your patch, setting lazy-highlight-max-at-a-time to, say, 2000,
> will still limit the highlighting to the displayed portion, which
> makes little sense to me, as the probability of finding more than 2000
> matches in a single window-full is practically zero, and so the user's
> intent will not be honored (and the doc string will still be
> misleading).

Yes.  As I said, more than once, the patch I submitted fixes the bug
only "for a nil value".  And I added:

  Feel free to adjust the fix.  Or to extend it to also take a
  numeric value into account.  But I will be happy if it is fixed
  at least for nil.

My immediate need, for which there is an easy, quick fix, is to make
the option work for nil.

I certainly agree that it would be good to also make it work for large
numeric values - which is why I invited such an additional fix.  That
will be useful.

> Instead, I suggest to use a special non-nil value, e.g. zero or -1, to
> indicate a limit to the current window's end, and treat any other
> value literally, disregarding the window limits.  (This will need to
> be reflected in the documentation, of course.)  That special value
> should IMO be the default.  Or maybe introduce a separate predicate
> option for whether to limit to window start and end.

That sounds OK to me.

Except that you did not explicitly mention nil.  Do you mean, by treat
it literally, that it should behave for nil as it is advertised now:
no limit at all?  If so then I agree.

IOW, I would like nil to behave as advertised: no limit.  This is the
use case that prompted me to file the bug and look for a solution.

> Yes, this would be a backward-incompatible change, but I don't think
> the difference will matter too much in most use cases, especially with
> the default value of lazy-highlight-cleanup.

Because this has never worked before for areas beyond the window, it
does not really present backward compatibility problems.  Unless you
are considering someone who has set the value to 99999999 and continues
to expect highlighting to be limited to the window.

> Btw, another issue that arises in this regard is whether to highlight
> beyond the screen only in the direction of the search (which AFAICS is
> what your proposed patch does) or in both directions, especially when
> the value of lazy-highlight-max-at-a-time is nil.

Good point.  My patch is not good enough here, though it is enough to
temporarily switch directions (e.g., from C-s to C-r) to work around
this incompleteness.

That is the first (next) thing to do, I think: fix things completely
for nil, so that it highlights all search hits (both directions).
I will take another look when I get a chance, if no one beats me
to it.

That would also give users a real difference in behavior between a
very large number and nil, which is good.  There should be a way,
which a very large number would provide, of getting a no-limit
behavior for only the current search direction.   (My patch
currently does this (for nil), but that is not the behavior nil
should provide, IMO.)

I appreciate your interest in this and hope that we do get it
fixed.