From: Alan Mackenzie <acm@muc.de>
To: Artur Malabarba <bruce.connor.am@gmail.com>
Cc: 17453@debbugs.gnu.org, emacs-devel <emacs-devel@gnu.org>,
Juri Linkov <juri@linkov.net>
Subject: bug#17453: Isearch doesn't work properly with Follow Mode.
Date: Mon, 2 Nov 2015 15:44:45 +0000 [thread overview]
Message-ID: <20151102154445.GD11804__44869.7187974632$1446479071$gmane$org@acm.fritz.box> (raw)
In-Reply-To: <CAAdUY-KbLvSEWayRu6CrMn-2bLTHd_m0nzzX4v7CXnWdo7P4sA@mail.gmail.com>
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
<buffer> 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 <bruce.connor.am@gmail.com>
> 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 <bruce.connor.am@gmail.com>
> 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 <bruce.connor.am@gmail.com>
> 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).
next prev parent reply other threads:[~2015-11-02 15:44 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-09 22:44 bug#17453: Isearch doesn't work properly with Follow Mode Alan Mackenzie
2014-05-10 2:40 ` Stefan Monnier
2014-05-11 12:58 ` Alan Mackenzie
2014-05-11 16:09 ` Stefan Monnier
2014-05-11 18:18 ` Alan Mackenzie
2014-05-11 19:05 ` Stefan Monnier
2014-05-11 20:40 ` Alan Mackenzie
2014-05-11 21:46 ` Stefan Monnier
2015-10-29 23:23 ` Alan Mackenzie
2015-10-31 22:35 ` John Wiegley
2015-10-31 22:35 ` John Wiegley
2015-10-31 23:25 ` Alan Mackenzie
2015-10-31 23:25 ` Alan Mackenzie
2015-10-31 23:41 ` John Wiegley
2015-11-01 11:59 ` Alan Mackenzie
2015-11-01 11:59 ` Alan Mackenzie
2015-10-31 23:41 ` John Wiegley
2015-11-01 0:17 ` Drew Adams
2015-11-01 0:17 ` Drew Adams
2015-10-31 23:13 ` Artur Malabarba
2015-10-31 23:32 ` Alan Mackenzie
2015-10-31 23:32 ` Alan Mackenzie
2015-11-01 12:20 ` Artur Malabarba
2015-11-01 12:23 ` Artur Malabarba
2015-11-01 13:52 ` Alan Mackenzie
2015-11-01 16:50 ` Eli Zaretskii
2015-11-01 18:27 ` Alan Mackenzie
2015-11-01 19:46 ` Artur Malabarba
2015-11-01 20:15 ` Alan Mackenzie
2015-11-01 21:37 ` Artur Malabarba
2015-11-01 20:42 ` Artur Malabarba
2015-11-01 20:54 ` Eli Zaretskii
2015-11-01 22:19 ` Alan Mackenzie
2015-11-01 12:23 ` Artur Malabarba
2015-11-01 12:20 ` Artur Malabarba
2015-10-31 23:13 ` Artur Malabarba
2015-10-31 23:35 ` Juri Linkov
2015-10-31 23:56 ` Alan Mackenzie
2015-11-02 0:14 ` Juri Linkov
2015-11-02 0:14 ` Juri Linkov
2015-11-02 3:35 ` Eli Zaretskii
2015-11-02 3:35 ` Eli Zaretskii
2015-11-02 9:28 ` Alan Mackenzie
2015-11-02 11:53 ` Artur Malabarba
2015-11-02 11:53 ` Artur Malabarba
2015-11-02 12:14 ` Artur Malabarba
2015-11-02 12:14 ` Artur Malabarba
2015-11-02 12:39 ` Alan Mackenzie
2015-11-02 12:39 ` Alan Mackenzie
2015-11-02 12:35 ` Alan Mackenzie
2015-11-02 13:10 ` Artur Malabarba
2015-11-02 14:18 ` Artur Malabarba
2015-11-02 14:18 ` Artur Malabarba
2015-11-02 15:44 ` Alan Mackenzie [this message]
2015-11-02 15:44 ` Alan Mackenzie
2015-11-02 16:26 ` Artur Malabarba
2015-11-02 16:35 ` Drew Adams
2015-11-02 19:18 ` Artur Malabarba
2015-11-02 19:28 ` Drew Adams
2015-11-02 23:45 ` Juri Linkov
2015-11-02 22:09 ` Alan Mackenzie
2015-11-02 23:00 ` Artur Malabarba
2015-11-03 9:18 ` Alan Mackenzie
2015-11-02 17:45 ` Eli Zaretskii
2015-11-02 23:22 ` Juri Linkov
2015-11-02 23:22 ` Juri Linkov
2015-11-03 12:31 ` Alan Mackenzie
2015-11-03 12:31 ` Alan Mackenzie
2015-11-03 15:49 ` Eli Zaretskii
2015-11-03 16:18 ` Artur Malabarba
2015-11-03 22:11 ` Alan Mackenzie
2015-11-04 0:28 ` Juri Linkov
2015-11-04 9:01 ` Alan Mackenzie
2015-11-04 10:17 ` Artur Malabarba
2015-11-05 12:38 ` Alan Mackenzie
2015-11-05 17:13 ` Artur Malabarba
2015-11-07 12:59 ` Alan Mackenzie
2015-11-07 13:38 ` Eli Zaretskii
2015-11-08 10:32 ` Alan Mackenzie
2015-11-03 16:39 ` Alan Mackenzie
2015-11-02 23:28 ` Juri Linkov
2015-11-02 23:28 ` Juri Linkov
2015-11-02 13:10 ` Artur Malabarba
2015-11-02 15:46 ` Eli Zaretskii
2015-11-02 16:09 ` Alan Mackenzie
2015-11-02 17:49 ` Eli Zaretskii
2015-11-02 20:35 ` John Wiegley
2015-11-03 8:35 ` Alan Mackenzie
[not found] ` <<831tc8xv39.fsf@gnu.org>
2015-11-02 16:05 ` Drew Adams
2015-11-02 12:35 ` Alan Mackenzie
2015-11-02 23:33 ` Juri Linkov
2015-11-02 23:33 ` Juri Linkov
2015-10-31 23:56 ` Alan Mackenzie
2015-10-31 23:35 ` Juri Linkov
2015-10-29 23:23 ` Alan Mackenzie
[not found] ` <handler.17453.B.139967578531952.ack@debbugs.gnu.org>
2015-12-20 12:59 ` bug#17453: Acknowledgement (Isearch doesn't work properly with Follow Mode.) Alan Mackenzie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='20151102154445.GD11804__44869.7187974632$1446479071$gmane$org@acm.fritz.box' \
--to=acm@muc.de \
--cc=17453@debbugs.gnu.org \
--cc=bruce.connor.am@gmail.com \
--cc=emacs-devel@gnu.org \
--cc=juri@linkov.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.