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: Mon, 2 Nov 2015 15:44:45 +0000 Message-ID: <20151102154445.GD11804__44869.7187974632$1446479071$gmane$org@acm.fritz.box> References: <20151029232302.GB3812@acm.fritz.box> <87h9l6627a.fsf@mail.linkov.net> <20151031235651.GE1853@acm.fritz.box> <87bnbddzpk.fsf@mail.linkov.net> <20151102092853.GA11804@acm.fritz.box> <20151102123512.GB11804@acm.fritz.box> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1446479071 19689 80.91.229.3 (2 Nov 2015 15:44:31 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 2 Nov 2015 15:44:31 +0000 (UTC) Cc: 17453@debbugs.gnu.org, emacs-devel , Juri Linkov To: Artur Malabarba Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Nov 02 16:44: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 1ZtHHN-0005rT-Lq for geb-bug-gnu-emacs@m.gmane.org; Mon, 02 Nov 2015 16:44:17 +0100 Original-Received: from localhost ([::1]:42971 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtHHN-0001Fm-5u for geb-bug-gnu-emacs@m.gmane.org; Mon, 02 Nov 2015 10:44:17 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34729) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtHHH-0001FN-6a for bug-gnu-emacs@gnu.org; Mon, 02 Nov 2015 10:44:13 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtHH8-0000v8-Uw for bug-gnu-emacs@gnu.org; Mon, 02 Nov 2015 10:44:11 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:59153) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtHH8-0000uZ-Ij for bug-gnu-emacs@gnu.org; Mon, 02 Nov 2015 10:44:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1ZtHH8-0006rr-5N for bug-gnu-emacs@gnu.org; Mon, 02 Nov 2015 10:44:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 02 Nov 2015 15:44: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.144647900126333 (code B ref 17453); Mon, 02 Nov 2015 15:44:02 +0000 Original-Received: (at 17453) by debbugs.gnu.org; 2 Nov 2015 15:43:21 +0000 Original-Received: from localhost ([127.0.0.1]:49861 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZtHGR-0006qc-Tk for submit@debbugs.gnu.org; Mon, 02 Nov 2015 10:43:20 -0500 Original-Received: from mail.muc.de ([193.149.48.3]:21962) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZtHG6-0006pp-Jd for 17453@debbugs.gnu.org; Mon, 02 Nov 2015 10:43:18 -0500 Original-Received: (qmail 6817 invoked by uid 3782); 2 Nov 2015 15:42:57 -0000 Original-Received: from acm.muc.de (p5B147B71.dip0.t-ipconnect.de [91.20.123.113]) by colin.muc.de (tmda-ofmipd) with ESMTP; Mon, 02 Nov 2015 16:42:56 +0100 Original-Received: (qmail 14043 invoked by uid 1000); 2 Nov 2015 15:44:45 -0000 Content-Disposition: inline In-Reply-To: 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:108301 Archived-At: Hello again, Artur On Mon, Nov 02, 2015 at 02:18:59PM +0000, Artur Malabarba wrote: > > > Lazy highlighting searches for matches on the current window. It must > > > be extended to search for matches on the Follow Mode group of windows. > > > For that, it needs the details of the "window*-start" and "window*-end". > > Yes, you're right, I see that now. This might still be solvable via > > setting isearch-search-fun-function, I'll need to think on that for a > > moment. > Here's a new version, split into 3 short patches to make it easier to > understand. Thanks. I understand what you're suggesting, now, and I think it's a good idea, on the whole. However, about the bug where, with lazy highlighting active, isearch scrolls the window its on rather than moving to the next one: as I said, this is caused by the "(sit-for 0)" near the beginning of isearch-lazy-highlight-new-loop. The purpose of this is to scroll the display, in case point has moved off of the window. The bug cause is now clear: ..-lazy-highlight-new-loop first scrolls the window, then checks whether it needs to restart its lazy h. loop. If it does, it starts the timer which triggers ..-lazy-highlight-update after the delay. The bug is that all this checking happens in ...-lazy-h-new-loop rather than in ...-lazy-h-update. If we moved all the checking to ..l-h-update, then there would be no need for "(sit-for 0)" - the command loop redisplay would already have scrolled the windows, if necessary; and Follow Mode would already have shifted point to the right window. So how about us just moving all these checks to where they really belong, in isearch-lazy-highlight-update? I've a feeling that if we do this, then your function follow--search-function becomes unneeded. Juri? ######################################################################### A related point in one of your patches below. It explicitly selects one of the Follow Mode windows. This is bad. Follow Mode itself choses the right window in its post-command-hook. Second-guessing it within the command processing is liable to lead to confusion, maintenance difficulties, and general pain. Another point. It is better to use follow-all-followers to get at the windows rather than get-buffer-window-list. The first of these is guaranteed to get the windows in the right order, and also allows for a future enhancement whereby only some of the windows displaying are part of the Follow Mode group. Of course, anything using follow-all-followers needs to be textually in follow.el. > This makes 2 small changes to isearch (which I think are even > justifiable on their own, regardless of follow-mode), and defines a > new function in follow-mode which takes care of two things: > Matches are highlighted on all windows Here, the matches should be highlighted on all the windows in the FM group without exception. I think a new option you're providing allows lazy highlighting to happen only in the current window. This is a bad idea when FM is active. > When the user types something, and the next match is on another > window, switch to that window instead of scrolling the current one. > From caf5adfc0b7caf07dc74813242f9ecc664babc13 Mon Sep 17 00:00:00 2001 > From: Artur Malabarba > Date: Mon, 2 Nov 2015 14:01:18 +0000 > Subject: [PATCH 3/3] * lisp/follow.el: Improve isearch compatibility > > (follow--search-function): New function. > (follow-mode): Configure `isearch-lazy-highlight' and > `isearch-search-fun-function'. > --- > lisp/follow.el | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/lisp/follow.el b/lisp/follow.el > index 938c59e..0b80f04 100644 > --- a/lisp/follow.el > +++ b/lisp/follow.el > @@ -419,6 +419,9 @@ follow-mode > :keymap follow-mode-map > (if follow-mode > (progn > + (setq-local isearch-search-fun-function #'follow--search-function) > + (when isearch-lazy-highlight > + (setq-local isearch-lazy-highlight 'all-windows)) > (add-hook 'compilation-filter-hook 'follow-align-compilation-windows t t) > (add-hook 'post-command-hook 'follow-post-command-hook t) > (add-hook 'window-size-change-functions 'follow-window-size-change t)) > @@ -434,6 +437,36 @@ follow-mode > (remove-hook 'window-size-change-functions 'follow-window-size-change))) > (remove-hook 'compilation-filter-hook 'follow-align-compilation-windows t))) > > +(defun follow--search-function () > + "Return a function suitable for `isearch-search-fun-function'." > + (lambda (string &optional bound noerror count) > + (let* ((search-function (isearch-search-fun-default)) > + (all-followers (follow-all-followers)) > + ;; If bound at the edge of the window, extend it to the > + ;; edges know by `follow-mode'. > + (bound (cond ((equal bound (window-end)) > + (window-end (car (last all-followers)))) > + ((equal bound (window-start)) > + (window-start (car all-followers))) > + (t bound))) > + (matched (funcall search-function string bound noerror count))) > + ;; If this is a proper user-triggered search (and not just a > + ;; lazy-highlight search), and if the search matched, and if the > + ;; match is not visible on this window... > + (when (and matched > + (not isearch-lazy-highlight-ongoing-search) > + (not (and (pos-visible-in-window-p (match-beginning 0)) > + (pos-visible-in-window-p (match-end 0))))) > + ;; ... see if the match is visible on another window. > + (let ((win (seq-find (lambda (w) > + (and (pos-visible-in-window-p (match-beginning 0) w) > + (pos-visible-in-window-p (match-end 0) w))) > + all-followers))) > + ;; If so, select it. > + (when win > + (select-window win)))) <=========================== > + matched))) > + > (defun follow-find-file-hook () > "Find-file hook for Follow mode. See the variable `follow-auto'." > (if follow-auto (follow-mode 1))) > -- > 2.6.2 > > From 8a52f12872d2b99acb2ca5a2077ccd87779309fe Mon Sep 17 00:00:00 2001 > From: Artur Malabarba > Date: Mon, 2 Nov 2015 13:54:15 +0000 > Subject: [PATCH 2/3] * lisp/isearch.el: Bind a variable while > lazy-highlighting > > (isearch-lazy-highlight-ongoing-search): New variable. > (isearch-lazy-highlight-search): Use it. > --- > lisp/isearch.el | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/lisp/isearch.el b/lisp/isearch.el > index 6442166..ef49650 100644 > --- a/lisp/isearch.el > +++ b/lisp/isearch.el > @@ -3080,11 +3080,18 @@ isearch-lazy-highlight-new-loop > (run-with-idle-timer lazy-highlight-initial-delay nil > 'isearch-lazy-highlight-update))))) > > +(defvar isearch-lazy-highlight-ongoing-search nil > + "Dynamically bound to t in `isearch-lazy-highlight-search'. > +This can be used by `isearch-search-fun-function' to detect > +whether its return value is being run for a proper user search or > +a lazy highlight search.") > + > (defun isearch-lazy-highlight-search () > "Search ahead for the next or previous match, for lazy highlighting. > Attempt to do the search exactly the way the pending Isearch would." > (condition-case nil > (let ((case-fold-search isearch-lazy-highlight-case-fold-search) > + (isearch-lazy-highlight-ongoing-search t) > (isearch-regexp isearch-lazy-highlight-regexp) > (isearch-regexp-function isearch-lazy-highlight-regexp-function) > (isearch-lax-whitespace > -- > 2.6.2 > > From 0c815fa21001b4a29f124e8eda58f7889560d701 Mon Sep 17 00:00:00 2001 > From: Artur Malabarba > Date: Mon, 2 Nov 2015 12:09:16 +0000 > Subject: [PATCH 1/3] * lisp/isearch.el: Allow lazy-highlighting of all windows > > (isearch-lazy-highlight): New possible value. > (isearch-lazy-highlight-update): Use it. > --- > lisp/isearch.el | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/lisp/isearch.el b/lisp/isearch.el > index b762884..6442166 100644 > --- a/lisp/isearch.el > +++ b/lisp/isearch.el > @@ -279,8 +279,13 @@ isearch-lazy-highlight > "Controls the lazy-highlighting during incremental search. > When non-nil, all text in the buffer matching the current search > string is highlighted lazily (see `lazy-highlight-initial-delay' > -and `lazy-highlight-interval')." > - :type 'boolean > +and `lazy-highlight-interval'). > + > +When multiple windows display the current buffer, the > +highlighting is displayed only on the selected window, unless > +this variable is set to the symbol `all-windows'." > + :type '(choice boolean > + (const :tag "On, and applied to all windows" all-windows)) > :group 'lazy-highlight > :group 'isearch) > > @@ -3162,7 +3167,8 @@ isearch-lazy-highlight-update > ;; but lower than isearch main overlay's 1001 > (overlay-put ov 'priority 1000) > (overlay-put ov 'face lazy-highlight-face) > - (overlay-put ov 'window (selected-window)))) > + (unless (eq isearch-lazy-highlight 'all-windows) > + (overlay-put ov 'window (selected-window))))) > ;; Remember the current position of point for > ;; the next call of `isearch-lazy-highlight-update' > ;; when `lazy-highlight-max-at-a-time' is too small. > -- > 2.6.2 > -- Alan Mackenzie (Nuremberg, Germany).