From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: bug#17453: Isearch doesn't work properly with Follow Mode. Date: Sun, 1 Nov 2015 13:52:53 +0000 Message-ID: <20151101135253.GB2768@acm.fritz.box> References: <20140509224458.GA4205@acm.acm> <20151029232302.GB3812@acm.fritz.box> <20151031233225.GD1853@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 1446385896 10444 80.91.229.3 (1 Nov 2015 13:51:36 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 1 Nov 2015 13:51:36 +0000 (UTC) Cc: emacs-devel To: Artur Malabarba Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Nov 01 14:51:27 2015 Return-path: Envelope-to: ged-emacs-devel@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 1Zst2c-0006as-Lr for ged-emacs-devel@m.gmane.org; Sun, 01 Nov 2015 14:51:26 +0100 Original-Received: from localhost ([::1]:36964 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zst2b-0007e3-Gm for ged-emacs-devel@m.gmane.org; Sun, 01 Nov 2015 08:51:25 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60576) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zst2O-0007dy-H7 for emacs-devel@gnu.org; Sun, 01 Nov 2015 08:51:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zst2J-0005j2-Az for emacs-devel@gnu.org; Sun, 01 Nov 2015 08:51:12 -0500 Original-Received: from mail.muc.de ([193.149.48.3]:38706) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zst2J-0005iy-0C for emacs-devel@gnu.org; Sun, 01 Nov 2015 08:51:07 -0500 Original-Received: (qmail 18204 invoked by uid 3782); 1 Nov 2015 13:51:05 -0000 Original-Received: from acm.muc.de (p548A4E61.dip0.t-ipconnect.de [84.138.78.97]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 01 Nov 2015 14:51:04 +0100 Original-Received: (qmail 3268 invoked by uid 1000); 1 Nov 2015 13:52:53 -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-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x X-Received-From: 193.149.48.3 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:193063 Archived-At: Hello, Artur. On Sun, Nov 01, 2015 at 12:23:52PM +0000, Artur Malabarba wrote: > 2015-11-01 12:20 GMT+00:00 Artur Malabarba : > > You have a better understanding of the issue and the code than I do, > > don't feel obliged to try my suggestion. > Anyway, could we see this code? :-) > Can you send it as patch or push it to a scratch/window-groups branch? No problem! The patch below doesn't actually apply at the moment, due to some (?very) recent change in isearch.el. But it wouldn't run anyway, because window*-start and friends aren't there yet. diff --git a/lisp/isearch.el b/lisp/isearch.el index 4fc9b38..07ec534 100644 --- a/lisp/isearch.el +++ b/lisp/isearch.el @@ -184,6 +184,11 @@ isearch-message-function "Function to call to display the search prompt. If nil, use function `isearch-message'.") +(defmacro isearch-call-message (&optional cqh ellip) + `(if isearch-message-function + (funcall isearch-message-function ,cqh ,ellip) + (isearch-message ,cqh ,ellip))) + (defvar isearch-wrap-function nil "Function to call to wrap the search when search is failed. If nil, move point to the beginning of the buffer for a forward search, @@ -615,6 +620,7 @@ isearch-adjusted (defvar isearch-slow-terminal-mode nil) ;; If t, using a small window. (defvar isearch-small-window nil) +(defvar isearch-suspended-follow-mode nil) ; only used with a small window. (defvar isearch-opoint 0) ;; The window configuration active at the beginning of the search. (defvar isearch-window-configuration nil) @@ -885,6 +891,7 @@ isearch-mode (abs search-slow-window-lines)))) isearch-other-end nil isearch-small-window nil + isearch-suspended-follow-mode nil isearch-just-started t isearch-start-hscroll (window-hscroll) @@ -971,14 +978,15 @@ isearch-update (null executing-kbd-macro)) (progn (if (not (input-pending-p)) - (if isearch-message-function - (funcall isearch-message-function) - (isearch-message))) + (isearch-call-message)) (if (and isearch-slow-terminal-mode (not (or isearch-small-window - (pos-visible-in-window-p)))) + (pos-visible-in-window*-p)))) (let ((found-point (point))) (setq isearch-small-window t) + (when (and (boundp 'follow-mode) follow-mode) + (follow-mode 0) + (setq isearch-suspended-follow-mode t)) (move-to-window-line 0) (let ((window-min-height 1)) (split-window nil (if (< search-slow-window-lines 0) @@ -997,7 +1005,7 @@ isearch-update (let ((current-scroll (window-hscroll)) visible-p) (set-window-hscroll (selected-window) isearch-start-hscroll) - (setq visible-p (pos-visible-in-window-p nil nil t)) + (setq visible-p (pos-visible-in-window*-p nil nil t)) (if (or (not visible-p) ;; When point is not visible because of hscroll, ;; pos-visible-in-window-p returns non-nil, but @@ -1051,17 +1059,20 @@ isearch-done (setq minibuffer-message-timeout isearch-original-minibuffer-message-timeout) (isearch-dehighlight) (lazy-highlight-cleanup lazy-highlight-cleanup) - (let ((found-start (window-start)) + (let ((found-start (window*-start)) (found-point (point))) (when isearch-window-configuration (set-window-configuration isearch-window-configuration) (if isearch-small-window - (goto-char found-point) + (progn + (when isearch-suspended-follow-mode + (follow-mode 1)) + (goto-char found-point)) ;; set-window-configuration clobbers window-start; restore it. ;; This has an annoying side effect of clearing the last_modiff ;; field of the window, which can cause unwanted scrolling, ;; so don't do it unless truly necessary. - (set-window-start (selected-window) found-start t)))) + (set-window*-start (selected-window) found-start t)))) (setq isearch-mode nil) (if isearch-input-method-local-p @@ -1284,13 +1295,6 @@ with-isearch-suspended (unwind-protect (progn ,@body) - ;; Set point at the start (end) of old match if forward (backward), - ;; so after exiting minibuffer isearch resumes at the start (end) - ;; of this match and can find it again. - (if (and old-other-end (eq old-point (point)) - (eq isearch-forward isearch-new-forward)) - (goto-char old-other-end)) - ;; Always resume isearching by restarting it. (isearch-mode isearch-forward isearch-regexp @@ -1303,7 +1307,17 @@ with-isearch-suspended isearch-message isearch-new-message isearch-forward isearch-new-forward isearch-word isearch-new-word - isearch-case-fold-search isearch-new-case-fold)) + isearch-case-fold-search isearch-new-case-fold) + + ;; Restore the minibuffer message before moving point. + (isearch-call-message nil t) + + ;; Set point at the start (end) of old match if forward (backward), + ;; so after exiting minibuffer isearch resumes at the start (end) + ;; of this match and can find it again. + (if (and old-other-end (eq old-point (point)) + (eq isearch-forward isearch-new-forward)) + (goto-char old-other-end))) ;; Empty isearch-string means use default. (when (= 0 (length isearch-string)) @@ -1898,6 +1912,7 @@ isearch-del-char (length isearch-string)))) isearch-message (mapconcat 'isearch-text-char-description isearch-string ""))) + (isearch-call-message nil t) ; Do this before moving point. ;; Use the isearch-other-end as new starting point to be able ;; to find the remaining part of the search string again. ;; This is like what `isearch-search-and-update' does, @@ -2074,6 +2089,7 @@ isearch-search-and-update (setq isearch-case-fold-search (isearch-no-upper-case-p isearch-string isearch-regexp)))) ;; Not regexp, not reverse, or no match at point. + (isearch-call-message nil t) ; Do this before moving point. (if (and isearch-other-end (not isearch-adjusted)) (goto-char (if isearch-forward isearch-other-end (min isearch-opoint @@ -2234,16 +2250,23 @@ isearch-allow-prefix :type 'boolean :group 'isearch) +;; (defmacro save-excursion-and-window (&rest body) +;; "FIXME!!!" +;; `(let ((-old-win- (selected-window))) +;; (save-excursion +;; (unwind-protect (progn ,@body) +;; (select-window -old-win- t))))) + (defun isearch-string-out-of-window (isearch-point) "Test whether the search string is currently outside of the window. Return nil if it's completely visible, or if point is visible, 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 (window*-start)) + (w-end (window*-end nil t)) + (w-L1 (save-selected-window (move-to-window*-line 1) (point))) + (w-L-1 (save-selected-window (move-to-window*-line -1) (point))) start end) ; start and end of search string in buffer (if isearch-forward (setq end isearch-point start (or isearch-other-end isearch-point)) @@ -2270,15 +2293,15 @@ isearch-back-into-window (if above (progn (goto-char start) - (recenter 0) - (when (>= isearch-point (window-end nil t)) + (recenter* 0) + (when (>= isearch-point (window*-end nil t)) (goto-char isearch-point) - (recenter -1))) + (recenter* -1))) (goto-char end) - (recenter -1) - (when (< isearch-point (window-start)) + (recenter* -1) + (when (< isearch-point (window*-start)) (goto-char isearch-point) - (recenter 0)))) + (recenter* 0)))) (goto-char isearch-point)) (defvar isearch-pre-scroll-point nil) @@ -2425,6 +2448,7 @@ isearch-ring-adjust (isearch-ring-adjust1 advance) (if search-ring-update (progn + (isearch-call-message nil t) (isearch-search) (isearch-push-state) (isearch-update)) @@ -2497,6 +2521,13 @@ isearch-complete-edit (defun isearch-message (&optional c-q-hack ellipsis) ;; Generate and print the message string. + + ;; N.B.: This function should always be called with point at the + ;; search point, because in certain (rare) circumstances, undesired + ;; scrolling can happen when point is elsewhere. These + ;; circumstances are when follow-mode is active, the search string + ;; spans two (or several) windows, and the message about to be + ;; displayed will cause the echo area to expand. (let ((cursor-in-echo-area ellipsis) (m isearch-message) (fail-pos (isearch-fail-pos t))) @@ -2679,9 +2710,6 @@ isearch-search-string (defun isearch-search () ;; Do the search with the current search string. - (if isearch-message-function - (funcall isearch-message-function nil t) - (isearch-message nil t)) (if (and (eq isearch-case-fold-search t) search-upper-case) (setq isearch-case-fold-search (isearch-no-upper-case-p isearch-string isearch-regexp))) @@ -3015,11 +3043,13 @@ isearch-lazy-highlight-new-loop search string to change or the window to scroll). It is also used by other Emacs features." (when (and (null executing-kbd-macro) - (sit-for 0) ;make sure (window-start) is credible + ;; make sure (window*-start) is credible and windows are + ;; properly synchronised. + (sit*-for 0) (or (not (equal isearch-string isearch-lazy-highlight-last-string)) - (not (eq (selected-window) - isearch-lazy-highlight-window)) + (not (memq (selected-window) + (window*-windows isearch-lazy-highlight-window))) (not (eq isearch-lazy-highlight-case-fold-search isearch-case-fold-search)) (not (eq isearch-lazy-highlight-regexp @@ -3030,9 +3060,9 @@ isearch-lazy-highlight-new-loop isearch-lax-whitespace)) (not (eq isearch-lazy-highlight-regexp-lax-whitespace isearch-regexp-lax-whitespace)) - (not (= (window-start) + (not (= (window*-start) isearch-lazy-highlight-window-start)) - (not (= (window-end) ; Window may have been split/joined. + (not (= (window*-end) ; Window may have been split/joined. isearch-lazy-highlight-window-end)) (not (eq isearch-forward isearch-lazy-highlight-forward)) @@ -3048,8 +3078,8 @@ isearch-lazy-highlight-new-loop (setq isearch-lazy-highlight-start-limit beg isearch-lazy-highlight-end-limit end) (setq isearch-lazy-highlight-window (selected-window) - isearch-lazy-highlight-window-start (window-start) - isearch-lazy-highlight-window-end (window-end) + isearch-lazy-highlight-window-start (window*-start) + isearch-lazy-highlight-window-end (window*-end) ;; Start lazy-highlighting at the beginning of the found ;; match (`isearch-other-end'). If no match, use point. ;; One of the next two variables (depending on search direction) @@ -3093,13 +3123,13 @@ isearch-lazy-highlight-search (+ isearch-lazy-highlight-start ;; Extend bound to match whole string at point (1- (length isearch-lazy-highlight-last-string))) - (window-end))) + (window*-end))) (max (or isearch-lazy-highlight-start-limit (point-min)) (if isearch-lazy-highlight-wrapped (- isearch-lazy-highlight-end ;; Extend bound to match whole string at point (1- (length isearch-lazy-highlight-last-string))) - (window-start)))))) + (window*-start)))))) ;; Use a loop like in `isearch-search'. (while retry (setq success (isearch-search-string @@ -3115,6 +3145,24 @@ isearch-lazy-highlight-search success) (error nil))) +(defun isearch-lazy-highlight-put-overlays (mb me) + "Put a highlighting overlay on the buffer for each pertinent window. + +An overlay is put over the positions (MB ME) in each window in the group +of windows (e.g. in Follow Mode) which (at least partially) contains them." + ;; non-zero-length match + (mapc (lambda (w) + (if (and (< mb (window-end w)) + (> me (window-start w))) + (let ((ov (make-overlay mb me))) + (push ov isearch-lazy-highlight-overlays) + ;; 1000 is higher than ediff's 100+, + ;; but lower than isearch main overlay's 1001 + (overlay-put ov 'priority 1000) + (overlay-put ov 'face lazy-highlight-face) + (overlay-put ov 'window w)))) + (window*-windows))) + (defun isearch-lazy-highlight-update () "Update highlighting of other matches for current search." (let ((max lazy-highlight-max-at-a-time) @@ -3143,23 +3191,15 @@ isearch-lazy-highlight-update (if isearch-lazy-highlight-forward (if (= mb (if isearch-lazy-highlight-wrapped isearch-lazy-highlight-start - (window-end))) + (window*-end))) (setq found nil) (forward-char 1)) (if (= mb (if isearch-lazy-highlight-wrapped isearch-lazy-highlight-end - (window-start))) + (window*-start))) (setq found nil) (forward-char -1))) - - ;; non-zero-length match - (let ((ov (make-overlay mb me))) - (push ov isearch-lazy-highlight-overlays) - ;; 1000 is higher than ediff's 100+, - ;; 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)))) + (isearch-lazy-highlight-put-overlays mb me)) ;; 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. @@ -3175,12 +3215,12 @@ isearch-lazy-highlight-update (setq isearch-lazy-highlight-wrapped t) (if isearch-lazy-highlight-forward (progn - (setq isearch-lazy-highlight-end (window-start)) + (setq isearch-lazy-highlight-end (window*-start)) (goto-char (max (or isearch-lazy-highlight-start-limit (point-min)) - (window-start)))) - (setq isearch-lazy-highlight-start (window-end)) + (window*-start)))) + (setq isearch-lazy-highlight-start (window*-end)) (goto-char (min (or isearch-lazy-highlight-end-limit (point-max)) - (window-end)))))))) + (window*-end)))))))) (unless nomore (setq isearch-lazy-highlight-timer (run-at-time lazy-highlight-interval nil @@ -3198,6 +3238,7 @@ isearch-resume (setq isearch-string string isearch-message message isearch-case-fold-search case-fold) + (isearch-call-message nil t) (isearch-search) (isearch-update)) -- Alan Mackenzie (Nuremberg, Germany).