GNU bug report logs - #17453
Isearch doesn't work properly with Follow Mode.

Previous Next

Full log

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

From: Alan Mackenzie <acm <at> muc.de>
To: Artur Malabarba <bruce.connor.am <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 17453 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#17453: Isearch doesn't work properly with Follow Mode.
Date: Thu, 5 Nov 2015 12:38:27 +0000

Hello, Artur.

On Wed, Nov 04, 2015 at 10:17:09AM +0000, Artur Malabarba wrote:
> 2015-11-04 9:01 GMT+00:00 Alan Mackenzie <acm <at> muc.de>:
> >> It still might help to synchronise the windows from isearch-update-post-hook
> >> if we'll call it before calling isearch-lazy-highlight-new-loop with sit-for.

> > I still say, wait until we really need it before we do anything so
> > drastic.  As Eli noted, follow-post-command-hook is SLOW, SLOW, SLOW.
> > If we call it twice per command, it will be twice as slow.

> > Also, why is the "(sit-for 0)" there at all?  As its comment says, It
> > is there for one purpose, and one purpose only: it is so that
> > (window-start) is valid, and the check

> >     (not (= (window-start)
> >             isearch-lazy-highlight-window-start))

> > will work.  This check means exactly "has the window scrolled?"

> Have you tested redisplay-would-scroll-window-p in a fully folded
> org-mode buffer with 100 headlines where each headline has 100 lines
> of content?

Of course not.  :-)

By the way, what does Isearch do with such org-mode folded buffer?  Does
it unfold the content, ever?

> If you report that it works in this context and isn't slow then I'm ok
> with going with this solution. Like I said, as long as it doesn't
> cause regressions, this would be a fine refactoring to do in
> isearch.el. So we might as well apply it now and give any possible
> issues some time to surface (isearch is used by tons of people, so I'm
> sure they'll surface if any exist).

> While you're at it, if you could also refactor all those `(not (equal
> ...))' tests into a single `isearch--lazy-highlight-needs-update-p'
> function that would be very welcome (and if you do, do it as a
> separate commit before everything else so the other stuff is easy to
> revert separately).

I don't agree this is a good idea.  At the moment, all these tests are
in the same not too big defun that sets them.  This kind of keeps things
together - for instance if a new test was to be introduced, only this
one function would need amending.

> And if it looks like I'm saying "I'm fine with this" to everything
> here, that's because I am. It sounds like we're debating two fine
> options and bordering on bikeshedding. So I say: merge one and see how
> it goes.

I'm pretty much ready to do this.  But maybe you could answer my
question about unfolding org-mode buffers above, first.  Thanks!

> Cheers

-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

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