* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
@ 2018-11-20 19:47 Drew Adams
2018-11-20 21:10 ` Drew Adams
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2018-11-20 19:47 UTC (permalink / raw)
To: 33446; +Cc: Juri Linkov
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''
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
2018-11-20 19:47 bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today Drew Adams
@ 2018-11-20 21:10 ` Drew Adams
2018-11-20 23:14 ` Juri Linkov
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2018-11-20 21:10 UTC (permalink / raw)
To: 33446; +Cc: Juri Linkov
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
2018-11-20 21:10 ` Drew Adams
@ 2018-11-20 23:14 ` Juri Linkov
2018-11-20 23:47 ` Drew Adams
0 siblings, 1 reply; 6+ messages in thread
From: Juri Linkov @ 2018-11-20 23:14 UTC (permalink / raw)
To: Drew Adams; +Cc: 33446
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
2018-11-20 23:14 ` Juri Linkov
@ 2018-11-20 23:47 ` Drew Adams
2018-11-21 22:42 ` Juri Linkov
0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2018-11-20 23:47 UTC (permalink / raw)
To: Juri Linkov; +Cc: 33446
> > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
2018-11-20 23:47 ` Drew Adams
@ 2018-11-21 22:42 ` Juri Linkov
2018-11-21 22:50 ` Drew Adams
0 siblings, 1 reply; 6+ messages in thread
From: Juri Linkov @ 2018-11-21 22:42 UTC (permalink / raw)
To: Drew Adams; +Cc: 33446-done
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today
2018-11-21 22:42 ` Juri Linkov
@ 2018-11-21 22:50 ` Drew Adams
0 siblings, 0 replies; 6+ messages in thread
From: Drew Adams @ 2018-11-21 22:50 UTC (permalink / raw)
To: Juri Linkov; +Cc: 33446-done
> > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-21 22:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-20 19:47 bug#33446: 26; `isearch-lazy-highlight-update' in MASTER today Drew Adams
2018-11-20 21:10 ` Drew Adams
2018-11-20 23:14 ` Juri Linkov
2018-11-20 23:47 ` Drew Adams
2018-11-21 22:42 ` Juri Linkov
2018-11-21 22:50 ` Drew Adams
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).