From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#17453: Isearch doesn't work properly with Follow Mode. Date: Sun, 11 May 2014 12:09:38 -0400 Message-ID: References: <20140509224458.GA4205@acm.acm> <20140511125842.GA11781@acm.acm> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1399824629 32271 80.91.229.3 (11 May 2014 16:10:29 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 11 May 2014 16:10:29 +0000 (UTC) Cc: 17453@debbugs.gnu.org To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun May 11 18:10:21 2014 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 1WjWKT-0006eQ-89 for geb-bug-gnu-emacs@m.gmane.org; Sun, 11 May 2014 18:10:21 +0200 Original-Received: from localhost ([::1]:33421 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjWKS-0006NT-Lo for geb-bug-gnu-emacs@m.gmane.org; Sun, 11 May 2014 12:10:20 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37062) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjWKI-0006NM-HY for bug-gnu-emacs@gnu.org; Sun, 11 May 2014 12:10:18 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjWKB-0003Pv-1U for bug-gnu-emacs@gnu.org; Sun, 11 May 2014 12:10:10 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:42096) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjWKA-0003Ow-VB for bug-gnu-emacs@gnu.org; Sun, 11 May 2014 12:10:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1WjWKA-0006bL-Bn for bug-gnu-emacs@gnu.org; Sun, 11 May 2014 12:10:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 11 May 2014 16:10:02 +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.139982458825349 (code B ref 17453); Sun, 11 May 2014 16:10:02 +0000 Original-Received: (at 17453) by debbugs.gnu.org; 11 May 2014 16:09:48 +0000 Original-Received: from localhost ([127.0.0.1]:59447 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjWJv-0006an-6u for submit@debbugs.gnu.org; Sun, 11 May 2014 12:09:47 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:30184) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjWJt-0006aV-1K for 17453@debbugs.gnu.org; Sun, 11 May 2014 12:09:45 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ArYGAIDvNVNMCqwB/2dsb2JhbABZgwaDSr0vgw6BFxd0giUBAQEBAgFWIwULCw4mEhQYDSSIBAjSGReOegeEOASpGYFqg0wh X-IPAS-Result: ArYGAIDvNVNMCqwB/2dsb2JhbABZgwaDSr0vgw6BFxd0giUBAQEBAgFWIwULCw4mEhQYDSSIBAjSGReOegeEOASpGYFqg0wh X-IronPort-AV: E=Sophos;i="4.97,753,1389762000"; d="scan'208";a="62342447" Original-Received: from 76-10-172-1.dsl.teksavvy.com (HELO pastel.home) ([76.10.172.1]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 11 May 2014 12:09:39 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 0EA17602A0; Sun, 11 May 2014 12:09:39 -0400 (EDT) In-Reply-To: <20140511125842.GA11781@acm.acm> (Alan Mackenzie's message of "Sun, 11 May 2014 12:58:42 +0000") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) 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: 140.186.70.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:88900 Archived-At: > Follow Mode knows nothing of Isearch. follow-mode.el doesn't, indeed, but since you're moving some follow mode code to isearch.el, that means that follow mode (which is now spread over follow-mode.el and isearch.el) knows something about Isearch. > The problem the other way round is in Lisp files that themselves play > with redisplay (including calling `sit-for') and `set-window-start', > and so on. Between these invocations and the actual redisplay, Follow > Mode must get a chance to make its adjustments. There's no doubt that follow mode's task is a tricky one, and I'm willing to add special support for it. I just want this special support to be a bit more generic than what you provided. It shouldn't be too difficult. It's just a matter of refactoring: change your patch so that on isearch.el's side it only adds some hooks, which are then set follow-mode.el. >> IOW we should try harder to come up with more general hooks. > For what? So that the same hooks can be used by other code than Isearch, for example. > Maybe a useful hook here would be `redisplay-hook' where Follow Mode > could do its stuff more effectively than on `post-command-hook'. There's pre-redisplay-function, which I think it does get us closer but is not sufficient yet. And of course, it won't help with things like "bring-region-into-sight". >> > @@ -2207,10 +2239,12 @@ >> > together with as much of the search string as will fit; the symbol >> > `above' if we need to scroll the text downwards; the symbol `below', >> > if upwards." >> > - (let ((w-start (window-start)) >> > - (w-end (window-end nil t)) >> > - (w-L1 (save-excursion (move-to-window-line 1) (point))) >> > - (w-L-1 (save-excursion (move-to-window-line -1) (point))) >> > + (let ((w-start (isearch-windows-start)) >> > + (w-end (isearch-windows-end nil t)) >> > + (w-L1 (with-selected-window (car isearch-windows) >> > + (save-excursion (move-to-window-line 1) (point)))) >> > + (w-L-1 (with-selected-window (car (last isearch-windows)) >> > + (save-excursion (move-to-window-line -1) (point)))) >> This isearch-string-out-of-window+isearch-back-into-window business, for >> example should be generalized to a function along the lines of >> "bring-region-into-sight". It's not useful only for isearch. > This seems to be a different bug to the one I reported, and orthogonal > to it. What bug? I'm just pointing out that the code you're modifying is more generally useful and that this generality is a good guide to help us decide where to put needed hooks. >> E.g. ediff-next-hunk would also benefit from it. ^^^^^ I meant "diff", sorry. Tho it would also be useful to ediff and smerge-mode as well, indeed. > Not necessarily. isearch-back-into-window, in addition to doing > "bring-region-into-sight" ensures that it's the isearch-point end of it > which is visible when it is too big for the window. This may be > problematic for other uses, like in ediff. I doubt this will be problematic. It seems quite natural for "bring-region-into-sight" to take as arguments not just the region but also some indication of the part to favor in case it can't all be displayed. > Question is, what about the main bug? The patch I wrote fixes a real > bug, and works (modulo any remaining bugs). Do you have any concrete > suggestions as to how to improve the fix? Hmm... I'm sorry I got lost along the way. I thought the whole patch is the bug fix (and the bring-region-into-sight part does seem like a bug-fix as well, tho maybe of top priority, admittedly). Could you show which part of the patch addresses the main bug? Stefan