GNU bug report logs - #33446
26; `isearch-lazy-highlight-update' in MASTER today

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 33446 in the body.
You can then email your comments to 33446 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
Cc: Juri Linkov <juri <at> linkov.net>
Subject: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Tue, 20 Nov 2018 11:47:29 -0800 (PST)

I ran into this bug today using my setup, which reflects the code for
`isearch-lazy-highlight-update' that was added recently to isearch.el.

Dunno why or how, but for some reason I got in a situation where
`isearch-lazy-highlight-window' is live and is not the selected window.
In fact it is in a different frame.  And there is no lazy-highlighting
showing there and no search in progress there.  I'm willing to guess
that this is not a normal situation that I got myself into. ;-)  But
I think there is nevertheless a bug somewhere in the Isearch code.

The `isearch-lazy-highlight-update' code does this:

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

That causes the window in the other frame to be selected.
It's not there that I wanted to do
`isearch-lazy-highlight-update'.  If there is no lazy
highlighting to be updated in the selected window then
I'd expect that `isearch-lazy-highlight-update' would in
that case be a no-op.  Why do we move to another window
to do it, even if for some reason `isearch-lazy-highlight-window'
is non-nil?  (I don't know why it is non-nil in my case.)

The code then does this:

  (setq window-start (window-group-start))
  (setq window-end (window-group-end))

This causes `window-end' to be nil.  Should `window-group-*' here
be passed window `isearch-lazy-highlight-window'?

In any case, then the code does this:

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

and that raises an error because `window-end' is nil.

Dunno what the right fix is.  Definitely min and max should not be
called if one of their args is nil.  What should be done in this case?

And I wonder, even if that error gets prevented, why we should update
lazy highlighting on a window other than the selected one?  Is that TRT?
In this case, at least, that other window has nothing to do with the
action in question (my code that calls `isearch-lazy-highlight-update'),
and there is no search in progress in that other window and no
lazy-highlighting (e.g. overlay) showing there.


In GNU Emacs 26.1 (build 1, x86_64-w64-mingw32)
 of 2018-05-30
Repository revision: 07f8f9bc5a51f5aa94eb099f3e15fbe0c20ea1ea
Windowing system distributor `Microsoft Corp.', version 10.0.16299
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

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

From: Drew Adams <drew.adams <at> oracle.com>
To: 33446 <at> debbugs.gnu.org
Cc: Juri Linkov <juri <at> linkov.net>
Subject: RE: bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Tue, 20 Nov 2018 13:10:50 -0800 (PST)

The proximate cause of the error was likely due to
my calling `isearch-lazy-highlight-update' outside
searching (i.e., nil `isearch-mode').  But I think
there might still be a bug in the Isearch code.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 33446 <at> debbugs.gnu.org
Subject: Re: bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Wed, 21 Nov 2018 01:14:12 +0200

> The proximate cause of the error was likely due to
> my calling `isearch-lazy-highlight-update' outside
> searching (i.e., nil `isearch-mode').  But I think
> there might still be a bug in the Isearch code.

isearch-lazy-highlight-update is not intended
to be used in a window other than the window
where isearch-lazy-highlight-new-loop was called.
It has many dependencies on the original window
being selected.

> The `isearch-lazy-highlight-update' code does this:
>
> (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))
>
> That causes the window in the other frame to be selected.

It should select the same window where
isearch-lazy-highlight-new-loop was called.

> The code then does this:
>
>   (setq window-start (window-group-start))
>   (setq window-end (window-group-end))
>
> This causes `window-end' to be nil.  Should `window-group-*' here
> be passed window `isearch-lazy-highlight-window'?

isearch-lazy-highlight-window should not be passed here
because it's guaranteed to be selected by the code above.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33446 <at> debbugs.gnu.org
Subject: RE: bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Tue, 20 Nov 2018 15:47:53 -0800 (PST)

> > The proximate cause of the error was likely due to
> > my calling `isearch-lazy-highlight-update' outside
> > searching (i.e., nil `isearch-mode').  But I think
> > there might still be a bug in the Isearch code.
> 
> isearch-lazy-highlight-update is not intended
> to be used in a window other than the window
> where isearch-lazy-highlight-new-loop was called.
> It has many dependencies on the original window
> being selected.

Yes, I guessed that finally.  Beyond that, I think it's
not intended to be invoked outside of search (i.e.,
when `isearch-mode' is nil).  That was my mistake:
invoking it in a command that can be used during Isearch
or not.

I know that wasn't intended as part of the Isearch
design.  Now I test for it and avoid calling `i-l-h-u'.

(But it might not hurt for `i-l-h-u' itself to test and
do nothing if `isearch-mode' is nil.  I'm not requesting
that, however.)

> > The `isearch-lazy-highlight-update' code does this:
> >
> > (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))
> >
> > That causes the window in the other frame to be selected.
> 
> It should select the same window where
> isearch-lazy-highlight-new-loop was called.

Which assumes it was called ;-) _and_ that we are still
within Isearch.

> > The code then does this:
> >
> >   (setq window-start (window-group-start))
> >   (setq window-end (window-group-end))
> >
> > This causes `window-end' to be nil.  Should `window-group-*' here
> > be passed window `isearch-lazy-highlight-window'?
> 
> isearch-lazy-highlight-window should not be passed here
> because it's guaranteed to be selected by the code above.

OK (but see above).

Feel free to close this bug, if you like.  It's not
clear to me whether any code change is needed.

Message #21 received at 33446-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 33446-done <at> debbugs.gnu.org
Subject: Re: bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Thu, 22 Nov 2018 00:42:34 +0200

> Yes, I guessed that finally.  Beyond that, I think it's
> not intended to be invoked outside of search (i.e.,
> when `isearch-mode' is nil).  That was my mistake:
> invoking it in a command that can be used during Isearch
> or not.
>
> I know that wasn't intended as part of the Isearch
> design.  Now I test for it and avoid calling `i-l-h-u'.
>
> (But it might not hurt for `i-l-h-u' itself to test and
> do nothing if `isearch-mode' is nil.  I'm not requesting
> that, however.)

isearch-lazy-highlight-update can't test for non-nil
isearch-mode, because it's also used in non-isearch cases
like in query-replace where isearch-mode is nil.

> Feel free to close this bug, if you like.  It's not
> clear to me whether any code change is needed.

I thought that your request was for using
isearch-lazy-highlight-window as an argument
of window functions in isearch-lazy-highlight-update.
If you need this, then please create a new request,
and I'm closing this one.

Message #24 received at 33446-done <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33446-done <at> debbugs.gnu.org
Subject: RE: bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
Date: Wed, 21 Nov 2018 14:50:56 -0800 (PST)

> > Yes, I guessed that finally.  Beyond that, I think it's
> > not intended to be invoked outside of search (i.e.,
> > when `isearch-mode' is nil).  That was my mistake:
> > invoking it in a command that can be used during Isearch
> > or not.
> >
> > I know that wasn't intended as part of the Isearch
> > design.  Now I test for it and avoid calling `i-l-h-u'.
> >
> > (But it might not hurt for `i-l-h-u' itself to test and
> > do nothing if `isearch-mode' is nil.  I'm not requesting
> > that, however.)
> 
> isearch-lazy-highlight-update can't test for non-nil
> isearch-mode, because it's also used in non-isearch cases
> like in query-replace where isearch-mode is nil.

Oh, right.  Is there some other state it can test
for?  As you said, it can only be expected to be
used (i.e., to work) when within an appropriate
context (e.g. following xyz).

> > Feel free to close this bug, if you like.  It's not
> > clear to me whether any code change is needed.
> 
> I thought that your request was for using
> isearch-lazy-highlight-window as an argument
> of window functions in isearch-lazy-highlight-update.
> If you need this, then please create a new request,
> and I'm closing this one.

That's fine.  Thx.

Previous Next

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