From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#17453: Isearch doesn't work properly with Follow Mode. Date: Wed, 4 Nov 2015 09:01:06 +0000 Message-ID: <20151104090106.GA1736@acm.fritz.box> References: <20151102123512.GB11804@acm.fritz.box> <20151102154445.GD11804@acm.fritz.box> <87h9l46l7o.fsf@mail.linkov.net> <20151103123116.GD2258@acm.fritz.box> <83a8qvw0aq.fsf@gnu.org> <20151103221131.GG2258@acm.fritz.box> <87mvuur4kn.fsf@mail.linkov.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1446627630 22561 80.91.229.3 (4 Nov 2015 09:00:30 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 4 Nov 2015 09:00:30 +0000 (UTC) Cc: 17453@debbugs.gnu.org, Artur Malabarba To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Nov 04 10:00:19 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZttvU-0003WO-Qo for geb-bug-gnu-emacs@m.gmane.org; Wed, 04 Nov 2015 10:00:17 +0100 Original-Received: from localhost ([::1]:53431 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZttvT-00063s-8Y for geb-bug-gnu-emacs@m.gmane.org; Wed, 04 Nov 2015 04:00:15 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZttvP-00061j-42 for bug-gnu-emacs@gnu.org; Wed, 04 Nov 2015 04:00:12 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZttvJ-000344-Sv for bug-gnu-emacs@gnu.org; Wed, 04 Nov 2015 04:00:10 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:33294) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZttvJ-00033w-Ph for bug-gnu-emacs@gnu.org; Wed, 04 Nov 2015 04:00:05 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1ZttvJ-0000hK-5Z for bug-gnu-emacs@gnu.org; Wed, 04 Nov 2015 04:00:05 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 04 Nov 2015 09:00:04 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 17453 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 17453-submit@debbugs.gnu.org id=B17453.14466275612601 (code B ref 17453); Wed, 04 Nov 2015 09:00:04 +0000 Original-Received: (at 17453) by debbugs.gnu.org; 4 Nov 2015 08:59:21 +0000 Original-Received: from localhost ([127.0.0.1]:52232 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Zttub-0000fs-2E for submit@debbugs.gnu.org; Wed, 04 Nov 2015 03:59:21 -0500 Original-Received: from mail.muc.de ([193.149.48.3]:55194) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZttuZ-0000fj-2W for 17453@debbugs.gnu.org; Wed, 04 Nov 2015 03:59:20 -0500 Original-Received: (qmail 46245 invoked by uid 3782); 4 Nov 2015 08:59:16 -0000 Original-Received: from acm.muc.de (p5B147F06.dip0.t-ipconnect.de [91.20.127.6]) by colin.muc.de (tmda-ofmipd) with ESMTP; Wed, 04 Nov 2015 09:59:15 +0100 Original-Received: (qmail 2400 invoked by uid 1000); 4 Nov 2015 09:01:06 -0000 Content-Disposition: inline In-Reply-To: <87mvuur4kn.fsf@mail.linkov.net> User-Agent: Mutt/1.5.23 (2014-03-12) X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:108423 Archived-At: Hello, Juri. On Wed, Nov 04, 2015 at 02:28:08AM +0200, Juri Linkov wrote: > >> >> This is complicated. Ideally, the Follow Mode windows should be > >> >> synchronised in FM's post-command-hook, not isearch's. It is not > >> >> isearch's job to realign windows. follow-post-command-hook both realigns > >> >> windows and choses an appropriate window to put point in. We should let > >> >> it. > >> > Once again, if some code in Isearch calls the same function that is > >> > used in follow-post-command-hook, the above is not an issue. > >> > Moreover, saving some calls to the hook will make Emacs more > >> > responsive. > >> I agree with Eli and Juri on this. If there's a solution as simple as > >> calling a follow-mode function in isearch-post-update-hook, then this > >> sounds like a no-downside solution. > > I'm wondering if we're still talking about the same problem. ;-) A > > simpler solution is _not_ to call a FM function from that Isearch hook. > > Unless we're talking at cross purposes, there is simply no need. As > > long as the Isearch command is allowed to go to completion without > > forcibly redisplaying, FM will re-synchronise the windows (if needed) > > and select an appropriate window for point, all on its own (in > > follow-post-command-hook). > 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?". > I see no problem in changing the order of calls to isearch-update-post-hook and > isearch-lazy-highlight-new-loop in isearch-update. Sure, follow-post-command-hook > will be called twice but at least this simple solution for follow-mode > doesn't require re-designing the whole lazy-highlighting machinery. In one of my mails yesterday, I proposed removing the (sit-for 0) and replacing this check on (window-start) with (redisplay-would-scroll-window-p) . This would fix the bug without any further changes. It would avoid any far reaching change in design of the lazy highlighting, avoid calling follow-post-command-hook twice, yet would work. > diff --git a/lisp/follow.el b/lisp/follow.el > index 938c59e..75c2788 100644 > --- a/lisp/follow.el > +++ b/lisp/follow.el > @@ -420,6 +420,7 @@ (define-minor-mode follow-mode > (if follow-mode > (progn > (add-hook 'compilation-filter-hook 'follow-align-compilation-windows t t) > + (add-hook 'isearch-update-post-hook 'follow-post-command-hook t t) > (add-hook 'post-command-hook 'follow-post-command-hook t) > (add-hook 'window-size-change-functions 'follow-window-size-change t)) > ;; Remove globally-installed hook functions only if there is no > @@ -432,6 +433,7 @@ (define-minor-mode follow-mode > (unless following > (remove-hook 'post-command-hook 'follow-post-command-hook) > (remove-hook 'window-size-change-functions 'follow-window-size-change))) > + (remove-hook 'isearch-update-post-hook 'follow-post-command-hook t) > (remove-hook 'compilation-filter-hook 'follow-align-compilation-windows t))) > (defun follow-find-file-hook () > diff --git a/lisp/isearch.el b/lisp/isearch.el > index b762884..1e4a1a5 100644 > --- a/lisp/isearch.el > +++ b/lisp/isearch.el > @@ -1011,12 +1011,12 @@ (defun isearch-update () > (setq ;; quit-flag nil not for isearch-mode > isearch-adjusted nil > isearch-yank-flag nil) > - (when isearch-lazy-highlight > - (isearch-lazy-highlight-new-loop)) > ;; We must prevent the point moving to the end of composition when a > ;; part of the composition has just been searched. > (setq disable-point-adjustment t) > - (run-hooks 'isearch-update-post-hook)) > + (run-hooks 'isearch-update-post-hook) > + (when isearch-lazy-highlight > + (isearch-lazy-highlight-new-loop))) > (defun isearch-done (&optional nopush edit) > "Exit Isearch mode. -- Alan Mackenzie (Nuremberg, Germany).