Should ending successful Isearch with C-g restore the relative window position?

Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

In a buffer with multiple occurrences of "the", position the window so that one
of the occurrences is near the top of the window.  Now do C-s the C-s C-s...

This successfully visits occurrences of "the", and if there are enough of them
then it scrolls the window to do this.  Now hit C-g to exit Isearch.

Point is moved back to its starting position (good).  But now that position,
instead of being near the window top, is in the center of the window.
`goto-char' has, in effect, recentered the point in the window.

To me, this is a misfeature.  I would much rather have the original window
position, as well as point, restored.  That is, I would rather see the cursor in
the same relative window location as it was before searching.

Is there some advantage of the current behavior that I'm missing?  Am I the only
one who thinks that restoring the window position along with point would be
better?

Re: Should ending successful Isearch with C-g restore the relative window position?

[Dani Moncayo]

> In a buffer with multiple occurrences of "the", position the window so that one
> of the occurrences is near the top of the window.  Now do C-s the C-s C-s...
>
> This successfully visits occurrences of "the", and if there are enough of them
> then it scrolls the window to do this.  Now hit C-g to exit Isearch.
>
> Point is moved back to its starting position (good).  But now that position,
> instead of being near the window top, is in the center of the window.
> `goto-char' has, in effect, recentered the point in the window.
>
> To me, this is a misfeature.  I would much rather have the original window
> position, as well as point, restored.  That is, I would rather see the cursor in
> the same relative window location as it was before searching.
>
> Is there some advantage of the current behavior that I'm missing?  Am I the only
> one who thinks that restoring the window position along with point would be
> better?

IMO, TRT would be to restore both point and the window position.  I
filed bug #12799, which is about a similar issue:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12799

-- 
Dani Moncayo

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

> IMO, TRT would be to restore both point and the window position.  I
> filed bug #12799, which is about a similar issue:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12799

I see.  Perhaps my setting of zero for `scroll-conservatively' is the culprit.

(But I see the same behavior in older Emacs versions, where the doc of
`scroll-conservatively' does not mention that zero means recenter.  Perhaps that
just represents an old doc problem.)

In my case, I really only care about this for Isearch.  Isearch should, I think,
bring you back where you were, including the window position.

Re: Should ending successful Isearch with C-g restore the relative window position?

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Mon, 14 Jan 2013 15:17:51 -0800
> Cc: emacs-devel@gnu.org
> 
> > IMO, TRT would be to restore both point and the window position.  I
> > filed bug #12799, which is about a similar issue:
> > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12799
> 
> I see.  Perhaps my setting of zero for `scroll-conservatively' is the culprit.
> 
> (But I see the same behavior in older Emacs versions, where the doc of
> `scroll-conservatively' does not mention that zero means recenter.  Perhaps that
> just represents an old doc problem.)

Recentering is the default, always was.

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

> Recentering is the default, always was.

Just the _default_?

Does that mean that there is a simple user setting that changes the behavior
from the default so that it returns things as they were before searching?

Re: Should ending successful Isearch with C-g restore the relative window position?

[Alan Mackenzie]

Hi, Drew.

On Tue, Jan 15, 2013 at 06:58:49AM -0800, Drew Adams wrote:
> > Recentering is the default, always was.

> Just the _default_?

> Does that mean that there is a simple user setting that changes the
> behavior from the default so that it returns things as they were
> before searching?

No, I don't think it does.  You might be being a bit too pedantic here.
I think it would be easy enough to record the window position in isearch
for this.  But...

If C-g restores the window position, then sooner or later somebody's
going to want C-<space> to do the same.  Seems perfectly reasonable.
Except that would lead to complications in the implementation of the
mark and mark-ring, possibly intractable ones.

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: Should ending successful Isearch with C-g restore the relativewindow position?

[Drew Adams]

> > > Recentering is the default, always was.
> 
> > Just the _default_?  Does that mean that there is a simple
> > user setting that changes the behavior from the default so
> > that it returns things as they were before searching?
> 
> No, I don't think it does.  You might be being a bit too 
> pedantic here.

Dunno why you think that, but OK.  So what was meant then by "the default"?
What's the alternative behavior, and how does a user get it?

> I think it would be easy enough to record the window position 
> in isearch for this.

Is that the alternative for users to "the default" behavior - recoding Isearch?
Not a rhetorical question.  If I'm missing something I'd like to know what the
suggestion is.

> But... If C-g restores the window position, then sooner or
> later somebody's going to want C-<space> to do the same.
> Seems perfectly reasonable.

Why?  The doc makes clear that the intention of `C-g' (for successful search) is
to take you back where you were.  Including window position as part of where you
were would be a natural and reasonable extension for the `C-g' behavior.

I would not object if that were optional for users, if there is someone who
really thinks we should also keep the current behavior.

But what does any of this have to do with `C-SPC'?  Sorry, you've lost me.  The
question is about `C-g' in the case of search success.

Re: Should ending successful Isearch with C-g restore the relativewindow position?

[Alan Mackenzie]

Hello again, Drew.

On Tue, Jan 15, 2013 at 08:22:08AM -0800, Drew Adams wrote:
> > > > Recentering is the default, always was.

> > > Just the _default_?  Does that mean that there is a simple user
> > > setting that changes the behavior from the default so that it
> > > returns things as they were before searching?

> > No, I don't think it does.  You might be being a bit too pedantic
> > here.

> Dunno why you think that, but OK.  So what was meant then by "the
> default"?  What's the alternative behavior, and how does a user get it?

I think Eli meant nothing more than "what Emacs in general usually does,
when no other special action is taken", but he can confirm that himself
if he wants.  The alternative is carefully to scroll the contents of the
window to the desired position, something which is out of reach of normal
users.

> > I think it would be easy enough to record the window position in
> > isearch for this.

> Is that the alternative for users to "the default" behavior - recoding
> Isearch?  Not a rhetorical question.  If I'm missing something I'd like
> to know what the suggestion is.

Recoding isearch is certainly an alternative, but it's not for the faint
of heart.

> > But... If C-g restores the window position, then sooner or later
> > somebody's going to want C-<space> to do the same.  Seems perfectly
> > reasonable.

> Why?  The doc makes clear that the intention of `C-g' (for successful
> search) is to take you back where you were.  Including window position
> as part of where you were would be a natural and reasonable extension
> for the `C-g' behavior.

> I would not object if that were optional for users, if there is someone
> who really thinks we should also keep the current behavior.

> But what does any of this have to do with `C-SPC'?  Sorry, you've lost
> me.  The question is about `C-g' in the case of search success.

I suppose it's the way I use isearch.  I frequently use C-g to go back to
my starting position, but I also use C-<space> to do this (completing the
search in the process).  So, to me, C-g and C-<space> are semantically
very close.  Were I a mere user, I'd get somewhat irritated if C-g
restored the original scroll-position, but C-<space> didn't.

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: Should ending successful Isearch with C-g restore therelativewindow position?

[Drew Adams]

> > But what does any of this have to do with `C-SPC'?  Sorry, 
> > you've lost me.  The question is about `C-g' in the case
> > of search success.
> 
> I suppose it's the way I use isearch.  I frequently use C-g 
> to go back to my starting position, but I also use C-<space>
> to do this (completing the search in the process).  So, to me,
> C-g and C-<space> are semantically very close.  Were I a mere
> user, I'd get somewhat irritated if C-g restored the original
> scroll-position, but C-<space> didn't.

Is this with emacs -Q or some special setting?  For me with emacs -Q, C-SPC
exits Isearch, setting the mark, and nothing more.

Yes, I can then pop the mark to get back where I was, but C-SPC itself does not
do anything like what C-g does, AFAICT.

Anyway, I get your point; thanks.

Re: Should ending successful Isearch with C-g restore the relative window position?

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <dmoncayo@gmail.com>, <emacs-devel@gnu.org>
> Date: Tue, 15 Jan 2013 06:58:49 -0800
> 
> > Recentering is the default, always was.
> 
> Just the _default_?
> 
> Does that mean that there is a simple user setting that changes the behavior
> from the default so that it returns things as they were before searching?

I doubt that.  You could try scroll-preserve-screen-position, but it
doesn't do a perfect job, and I'm not even sure it will change
anything in the situation you described.

What I meant by "is the default, always was" is that the recentering
behavior was originally the only one available, so it predates any
scroll-* variables; therefore, it is small wonder that you find this
behavior in old versions of Emacs, where the scroll-* variables aren't
available or behave differently.

The non-default behavior, triggered by customizing some of the
scroll-* variables, can put point on the first or the last window line
(depending on the direction of movement), or make sure point is never
too close to the window edge.  But that's about all.  Emacs was never
programmed to predictably return to the same place on the screen (and
it isn't easy, either, due to variable fonts).

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

> The non-default behavior, triggered by customizing some of the
> scroll-* variables, can put point on the first or the last window line
> (depending on the direction of movement), or make sure point is never
> too close to the window edge.  But that's about all.  Emacs was never
> programmed to predictably return to the same place on the screen (and
> it isn't easy, either, due to variable fonts).

Thanks for the explanation.  So it sounds like this is not an enhancement we can
expect anytime soon.

To be clear - for me at least, getting back to something closer to where I was
before searching (window position included) would be an improvement, even if it
were not 100% perfect because of variable-width fonts.  (I don't use
variable-width fonts that much anyway, personally.)

Re: Should ending successful Isearch with C-g restore the relative window position?

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <dmoncayo@gmail.com>, <emacs-devel@gnu.org>
> Date: Tue, 15 Jan 2013 08:58:40 -0800
> 
> Thanks for the explanation.  So it sounds like this is not an enhancement we can
> expect anytime soon.

Depends on the availability of motivated volunteers ;-)

> To be clear - for me at least, getting back to something closer to where I was
> before searching (window position included) would be an improvement, even if it
> were not 100% perfect because of variable-width fonts.  (I don't use
> variable-width fonts that much anyway, personally.)

Does scroll-preserve-screen-position change anything in this
situation?

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

> Does scroll-preserve-screen-position change anything in this
> situation?

Not for me.  Does it help for you?

Also, I do not want scrolling to preserve screen position, according to what the
option description says that means.  I just want Isearch C-g to return me to
where I was (including window position) after successful search.

That has little to do with "scrolling" per se, AFAICT.  Yes, the text gets
"scrolled", in a sense, when Isearch moves to a match that is outside the text
shown in the window.  But that is not what I think of as scrolling.

Anyway, reading the option description does not lead me to conclude that it
would act also for such Isearch "scrolling".  And it does not, for me at least.

Re: Should ending successful Isearch with C-g restore the relative window position?

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <dmoncayo@gmail.com>, <emacs-devel@gnu.org>
> Date: Tue, 15 Jan 2013 09:59:02 -0800
> 
> > Does scroll-preserve-screen-position change anything in this
> > situation?
> 
> Not for me.  Does it help for you?

Didn't try.  But I suspected that much.

> That has little to do with "scrolling" per se, AFAICT.  Yes, the text gets
> "scrolled", in a sense, when Isearch moves to a match that is outside the text
> shown in the window.  But that is not what I think of as scrolling.

Alas, too many people expect Emacs to behave similarly to what it does
during scrolling, even if point moves for reasons entirely unrelated
to scroll commands.  So most, if not all, of the scroll-* variables
are in effect even if there's no scrolling anywhere in sight.

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

> > Thanks for the explanation.  So it sounds like this is not 
> > an enhancement we can expect anytime soon.
> 
> Depends on the availability of motivated volunteers ;-)

Well I'm certainly no expert here.  And I believe you when you say that it will
be difficult to fix this adequately.

I took a quick stab at it anyway.  Please try the attached patch, as a first
attempt.  It does not provide for the new behavior to be optional - dunno
whether that is needed.

I did not check the many possible Isearch code paths and use cases.  I just
checked it quickly.  So far, it seems to DTRT for the few things I tried.

If this happens to actually be TRT, great.  But the change I made is trivial, so
I assume there must be more to it, to DTRT in all cases.

I hope that passing this along might allow someone to enlighten me further or
perhaps encourage someone to provide a more complete solution.

All I did was this:

1. Save the relative window line number of point, before searching, in
`isearch-mode'.

2. Restore that, in `isearch-cancel'.

HTH.

[-- Attachment #2: isearch-2013-01-15.patch --]
[-- Type: application/octet-stream, Size: 1852 bytes --]

diff -c isearch-patched-2013-01-15.el isearch.el
*** isearch-patched-2013-01-15.el	Tue Jan 15 13:54:02 2013
--- isearch.el	Tue Jan 15 13:49:58 2013
***************
*** 605,612 ****
  (defvar isearch-opoint 0)
  ;; The window configuration active at the beginning of the search.
  (defvar isearch-window-configuration nil)
- ;; The line number of point before searching, relative to the line number of `window-start'.
- (defvar isearch-win-pt-line nil)
  
  ;; Flag to indicate a yank occurred, so don't move the cursor.
  (defvar isearch-yank-flag nil)
--- 605,610 ----
***************
*** 829,836 ****
  	isearch-start-hscroll (window-hscroll)
  
  	isearch-opoint (point)
-         isearch-win-pt-line (- (line-number-at-pos)
-                                (line-number-at-pos (window-start)))
  	search-ring-yank-pointer nil
  	isearch-opened-overlays nil
  	isearch-input-method-function input-method-function
--- 827,832 ----
***************
*** 1312,1319 ****
        ;; This calls pop-state function and restores original point.
        (let ((isearch-cmds (last isearch-cmds)))
  	(isearch--set-state (car isearch-cmds)))
!     (goto-char isearch-opoint)
!     (when isearch-win-pt-line (recenter isearch-win-pt-line)))
    (isearch-done t)                      ; Exit isearch..
    (isearch-clean-overlays)
    (signal 'quit nil))                   ; ..and pass on quit signal.
--- 1308,1314 ----
        ;; This calls pop-state function and restores original point.
        (let ((isearch-cmds (last isearch-cmds)))
  	(isearch--set-state (car isearch-cmds)))
!     (goto-char isearch-opoint))
    (isearch-done t)                      ; Exit isearch..
    (isearch-clean-overlays)
    (signal 'quit nil))                   ; ..and pass on quit signal.

Diff finished.  Tue Jan 15 20:54:28 2013

RE: Should ending successful Isearch with C-g restore therelative window position?

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

> All I did was this:
> 
> 1. Save the relative window line number of point, before searching, in
> `isearch-mode'.
> 
> 2. Restore that, in `isearch-cancel'.

That patch did not fix the problem when searching across Info nodes.
The attached patch does.  AFAICT, it works generally.

[-- Attachment #2: isearch-2013-01-16.patch --]
[-- Type: application/octet-stream, Size: 2061 bytes --]

diff -c isearch.el isearch-patched-2013-01-16.el
*** isearch.el	Wed Jan 16 08:49:58 2013
--- isearch-patched-2013-01-16.el	Wed Jan 16 08:51:10 2013
***************
*** 605,610 ****
--- 605,612 ----
  (defvar isearch-opoint 0)
  ;; The window configuration active at the beginning of the search.
  (defvar isearch-window-configuration nil)
+ ;; The line number of point before searching, relative to the line number of `window-start'.
+ (defvar isearch-win-pt-line nil)
  
  ;; Flag to indicate a yank occurred, so don't move the cursor.
  (defvar isearch-yank-flag nil)
***************
*** 827,832 ****
--- 829,836 ----
  	isearch-start-hscroll (window-hscroll)
  
  	isearch-opoint (point)
+         isearch-win-pt-line (- (line-number-at-pos)
+                                (line-number-at-pos (window-start)))
  	search-ring-yank-pointer nil
  	isearch-opened-overlays nil
  	isearch-input-method-function input-method-function
***************
*** 1307,1314 ****
        ;; For defined push-state function, restore the first state.
        ;; This calls pop-state function and restores original point.
        (let ((isearch-cmds (last isearch-cmds)))
! 	(isearch--set-state (car isearch-cmds)))
!     (goto-char isearch-opoint))
    (isearch-done t)                      ; Exit isearch..
    (isearch-clean-overlays)
    (signal 'quit nil))                   ; ..and pass on quit signal.
--- 1311,1320 ----
        ;; For defined push-state function, restore the first state.
        ;; This calls pop-state function and restores original point.
        (let ((isearch-cmds (last isearch-cmds)))
! 	(isearch--set-state (car isearch-cmds))
!         (when isearch-win-pt-line (recenter isearch-win-pt-line)))
!     (goto-char isearch-opoint)
!     (when isearch-win-pt-line (recenter isearch-win-pt-line)))
    (isearch-done t)                      ; Exit isearch..
    (isearch-clean-overlays)
    (signal 'quit nil))                   ; ..and pass on quit signal.

Diff finished.  Wed Jan 16 15:52:01 2013

Re: Should ending successful Isearch with C-g restore the relative window position?

[Juri Linkov]

> That patch did not fix the problem when searching across Info nodes.

A more general approach would be to save the window start position on
the stack and restore it in `isearch-cancel' like the patch below does.

`isearch-done' already does something like this by restoring
the original window configuration with `set-window-configuration'
from `isearch-window-configuration'.  But it does this only
on slow terminals where baud-rate is less than 1200.  So perhaps
this patch will also make `isearch-window-configuration' obsolete.

The remaining problem is how to take into account the case
when `isearch-allow-scroll' is non-nil and the user scrolled the
window-start to another position.  We should find the right place
to update the window-start of the previous state on the search stack
to be able to restore it later.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-01-02 16:13:04 +0000
+++ lisp/isearch.el	2013-01-16 21:28:52 +0000
@@ -1036,6 +1036,7 @@ (cl-defstruct (isearch--state
                  (string isearch-string)
                  (message isearch-message)
                  (point (point))
+                 (window-start (window-start))
                  (success isearch-success)
                  (forward isearch-forward)
                  (other-end isearch-other-end)
@@ -1049,6 +1050,7 @@ (cl-defstruct (isearch--state
   (string :read-only t)
   (message :read-only t)
   (point :read-only t)
+  (window-start :read-only t)
   (success :read-only t)
   (forward :read-only t)
   (other-end :read-only t)
@@ -1072,7 +1074,9 @@ (defun isearch--set-state (cmd)
 	isearch-case-fold-search (isearch--state-case-fold-search cmd))
   (if (functionp (isearch--state-pop-fun cmd))
       (funcall (isearch--state-pop-fun cmd) cmd))
-  (goto-char (isearch--state-point cmd)))
+  (goto-char (isearch--state-point cmd))
+  (unless (eq (isearch--state-window-start cmd) (window-start))
+    (set-window-start nil (isearch--state-window-start cmd))))
 
 (defun isearch-pop-state ()
   (setq isearch-cmds (cdr isearch-cmds))
@@ -1303,12 +1307,8 @@ (defun isearch-reverse-exit-minibuffer (
 (defun isearch-cancel ()
   "Terminate the search and go back to the starting point."
   (interactive)
-  (if (and isearch-push-state-function isearch-cmds)
-      ;; For defined push-state function, restore the first state.
-      ;; This calls pop-state function and restores original point.
-      (let ((isearch-cmds (last isearch-cmds)))
-	(isearch--set-state (car isearch-cmds)))
-    (goto-char isearch-opoint))
+  (let ((isearch-cmds (last isearch-cmds)))
+    (isearch--set-state (car isearch-cmds)))
   (isearch-done t)                      ; Exit isearch..
   (isearch-clean-overlays)
   (signal 'quit nil))                   ; ..and pass on quit signal.
@@ -2646,7 +2646,9 @@ (defun isearch-search ()
     (if (functionp (isearch--state-pop-fun (car isearch-cmds)))
         (funcall (isearch--state-pop-fun (car isearch-cmds))
                  (car isearch-cmds)))
-    (goto-char (isearch--state-point (car isearch-cmds)))))
+    (goto-char (isearch--state-point (car isearch-cmds)))
+    (unless (eq (isearch--state-window-start (car isearch-cmds)) (window-start))
+      (set-window-start nil (isearch--state-window-start (car isearch-cmds))))))
 
 

RE: Should ending successful Isearch with C-g restore the relative window position?

[Drew Adams]

> > That patch did not fix the problem when searching across Info nodes.
> > The attached patch does.  AFAICT, it works generally.
> 
> A more general approach would be to save the window start position
> on the stack and restore it in `isearch-cancel' like the patch 
> below does.

AFAICT, your slightly more complicated code has the same effect as mine.  Yes,
it adds `window-start' to the state struct, but I don't see any particular
advantage.  Is there some case it handles better?

> `isearch-done' already does something like this by restoring
> the original window configuration with `set-window-configuration'
> from `isearch-window-configuration'.  But it does this only
> on slow terminals where baud-rate is less than 1200.

That code was in fact my starting point, when looking at fixing this.

I almost filed an bug report (enhancement request), wondering whether we might
want the relative positions of point in each window to be saved as part of a
window configuration.  (Still wondering whether that might be a good idea.)

> So perhaps this patch will also make `isearch-window-configuration' obsolete.

I don't see how.  How is it relevant to what that code does?  That code does not
save/restore `window-start' or the window-relative cursor position, and your
patch (like mine) does not save/restore the window config.

(But I wonder whether that code might be obsolete anyway, in the sense of not
being used at all anymore.)

> The remaining problem is how to take into account the case
> when `isearch-allow-scroll' is non-nil and the user scrolled the
> window-start to another position.

I don't understand the problem.  With either my patch or yours, non-nil
`isearch-allow-scroll' seems to DTRT, AFAICT.  You can scroll using `C-v', mouse
wheel, etc. during Isearch, and if you hit `C-g' to cancel searching then you
get back where you started, including cursor position relative to window.  Works
everywhere I've tested, including across nodes in Info.

Can you give a recipe illustrating the problem you see?

> We should find the right place to update the window-start of the
> previous state on the search stack to be able to restore it later.

See above.  I don't understand what the problem is.  Seems to work OK as is.

Re: Should ending successful Isearch with C-g restore the relative window position?

[Juri Linkov]

> Can you give a recipe illustrating the problem you see?

I answered in the context of bug#12253.
Sorry if this too confusing, let's
continue this discussion in bug#12253.

Re: Should ending successful Isearch with C-g restore the relative window position?

[Juri Linkov]

>> Is there some advantage of the current behavior that I'm missing?
>> Am I the only one who thinks that restoring the window position along
>> with point would be better?
>
> IMO, TRT would be to restore both point and the window position.  I
> filed bug #12799, which is about a similar issue:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12799

And http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12253#32
so no new bug report is necessary to implement this enhancement.