bug#23621: 25.1.50; Buffer in >1 window; winner-undo recover window point

bug#23621: 25.1.50; Buffer in >1 window; winner-undo recover window point

[Tino Calancha]

For users editing same buffer in differente windows
it may be useful if `winner-undo' could restaure
also the window point from each window.

;; visit same file in 2 windows
./emacs -Q -mm -eval '(progn
                         (find-file-read-only "../src/window.c")
                         (winner-mode 1)
                         (goto-char 1000)
                         (split-window-right)
                         (other-window 1)
                         (goto-char 30500))'

M-! ls ~ RET ; C-x 0 ;; C-x 1 ;;; C-x b
C-c <left>

;; `window-point' return 30500 value for both windows.


;; You may want to preserve > 2 buffr positions.
;; Following example visits different positions of the same
;; buffer in 4 windows.
./emacs -Q -mm -eval '(progn
                         (find-file-read-only "../src/window.c")
                         (winner-mode 1)
                         (goto-char 1000)
                         (split-window-right)
                         (windmove-right 1)
                         (goto-char 30500)

                         (split-window-below)
                         (windmove-down 1)
                         (goto-char 50000)

                         (windmove-left 1)
                         (split-window-below)
                         (windmove-down 1)
                         (goto-char 250000))'

;; As before `window-point' is not preserved for all windows.


In GNU Emacs 25.1.50.3 (x86_64-pc-linux-gnu, GTK+ Version 2.24.30)
  of 2016-05-26 built on calancha-pc
Repository revision: 16be3e90545972dec16014253a843229d5bdf388

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Tino Calancha]

Allow users editing same buffer in diferent windows to restore
the window point with `winner-undo'.

This report is related with Bug#4041: the behaviour of 
`winner-undo' just change if an user has set the option 
`switch-to-buffer-preserve-window-point' non-nil.

Following patch restore the buffer point from deleted windows in the
2 previous examples (if `switch-to-buffer-preserve-window-point' evaluates
non-nil).

From 37c088a3cc0ae303dda570e4d93195131f6a9892 Mon Sep 17 00:00:00 2001
From: Tino Calancha <f92capac@gmail.com>
Date: Fri, 27 May 2016 01:27:18 +0900
Subject: [PATCH] Editing same buffer in >1 window; winner-undo enhancement

Allow winner-undo to restore the buffer point from deleted
windows (Bug#23621).
* lisp/window.el (window--before-delete-windows): New defun.
(delete-window, delete-other-windows): Use it.
* lisp/winner.el (winner-set): Use marker in 'window-prev-buffers'
when available and different than the value returned by 'winner-get-point'.
---
  lisp/window.el | 39 +++++++++++++++++++++++++++++++++++++++
  lisp/winner.el | 13 +++++++++++--
  2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index bd5275b..7a7f9ac 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -3775,6 +3775,41 @@ window--in-subtree-p
  		(throw 'done t)
  	      (setq parent (window-parent parent))))))))

+;; This function is called by `delete-window' and
+;; `delete-other-windows' when `switch-to-buffer-preserve-window-point'
+;; evaluates non-nil: it allows `winner-undo' to restore the
+;; buffer point from deleted windows (Bug#23621).
+(defun window--before-delete-windows (&optional window)
+  "Update `window-prev-buffers' before delete a window.
+Optional arg WINDOW, if non-nil, update WINDOW-START and POS
+in `window-prev-buffers' for all windows displaying same
+buffer as WINDOW.  Otherwise, update `window-prev-buffers' for
+all windows.
+
+The new values for WINDOW-START and POS are those
+returned by `window-start' and `window-point' respectively.
+
+This function is called only if `switch-to-buffer-preserve-window-point'
+evaluates non-nil."
+  (dolist (win (window-list))
+    (let* ((buf   (window-buffer (or window win)))
+           (start (window-start win))
+           (pos   (window-point win))
+           (entry (assq buf (window-prev-buffers win))))
+      (cond (entry
+             (let ((marker (nth 2 entry)))
+               (unless (= pos marker)
+                 (set-marker (nth 1 entry) start buf)
+                 (set-marker marker pos buf))))
+            (t
+             (let ((prev-buf (window-prev-buffers win))
+                   (start-m  (make-marker))
+                   (pos-m    (make-marker)))
+               (set-marker start-m start buf)
+               (set-marker pos-m pos buf)
+               (push (list buf start-m pos-m) prev-buf)
+               (set-window-prev-buffers win prev-buf)))))))
+
  (defun delete-window (&optional window)
    "Delete WINDOW.
  WINDOW must be a valid window and defaults to the selected one.
@@ -3793,6 +3828,8 @@ delete-window
  its frame, the last non-side window, or part of an atomic window
  that is its frame's root window."
    (interactive)
+  (when switch-to-buffer-preserve-window-point
+    (window--before-delete-windows window))
    (setq window (window-normalize-window window))
    (let* ((frame (window-frame window))
  	 (function (window-parameter window 'delete-window))
@@ -3875,6 +3912,8 @@ delete-other-windows
  on the frame.  Side windows are not deleted.  If WINDOW is a side
  window signal an error."
    (interactive)
+  (when switch-to-buffer-preserve-window-point
+    (window--before-delete-windows))
    (setq window (window-normalize-window window))
    (let* ((frame (window-frame window))
  	 (function (window-parameter window 'delete-other-windows))
diff --git a/lisp/winner.el b/lisp/winner.el
index 9a6f5d5..2a213ab 100644
--- a/lisp/winner.el
+++ b/lisp/winner.el
@@ -297,8 +297,17 @@ winner-set
        ;; Restore points
        (dolist (win (winner-sorted-window-list))
          (unless (and (pop alive)
-                     (setf (window-point win)
-                           (winner-get-point (window-buffer win) win))
+                     (let* ((buf   (window-buffer win))
+                            (pos   (winner-get-point (window-buffer win) win))
+                            (entry (assq buf (window-prev-buffers win))))
+                       ;; Try to restore point of buffer in the selected
+                       ;; window (Bug#23621).
+                       (let ((marker (nth 2 entry)))
+                         (when (and switch-to-buffer-preserve-window-point
+                                    marker
+                                    (not (= marker pos)))
+                           (setq pos marker))
+                         (setf (window-point win) pos)))
                       (not (member (buffer-name (window-buffer win))
                                    winner-boring-buffers)))
            (push win xwins)))            ; delete this window
-- 
2.8.1

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Lars Ingebrigtsen]

Tino Calancha <f92capac@gmail.com> writes:

> Allow users editing same buffer in diferent windows to restore
> the window point with `winner-undo'.

The patch looked OK to me, but I'm not a winner user.  I've applied the
patch and tested superficially, but if this commit leads to problems,
please revert the patch and reopen this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Michael Heerdegen]

Tino Calancha <f92capac@gmail.com> writes:

> Allow winner-undo to restore the buffer point from deleted
> windows (Bug#23621).
> * lisp/window.el (window--before-delete-windows): New defun.
> (delete-window, delete-other-windows): Use it.

Seems your patch works only partially because it actually changed
`minimize-window' instead of `delete-other-windows'.  Was it by mistake?

I also have a question: why do we need this at all?  Is it because the
window configuations saved are pushed when e.g. a window is split, but
only afterwards the user changes point etc, then a window is closed, and
when that's undone, without the patch, the originally saved window
configuration contains the initial, useless positions, because the
snapshot had actually been taken "too soon"?

If that is the case - we now have `compare-window-configurations' that
compares modulo value of point and such.  Wouldn't it be simpler to
teach delete-window e.a. to update the head window configuration in the
frame's entry of winner-ring-alist with the current version before a
window is deleted?


TIA,

Michael.

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> If that is the case - we now have `compare-window-configurations' that
> compares modulo value of point and such.  Wouldn't it be simpler to
> teach delete-window e.a. to update the head window configuration in the
> frame's entry of winner-ring-alist with the current version before a
> window is deleted?

Sorry, I missed this -- should 0454bfd3313c069ca395f02ab6f377a17ff44965
be reverted?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > If that is the case - we now have `compare-window-configurations' that
> > compares modulo value of point and such.  Wouldn't it be simpler to
> > teach delete-window e.a. to update the head window configuration in the
> > frame's entry of winner-ring-alist with the current version before a
> > window is deleted?
>
> Sorry, I missed this -- should 0454bfd3313c069ca395f02ab6f377a17ff44965
> be reverted?

I don't know much about this stuff.  Would be nice to hear from Tino.

Regards,

Michael.

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[martin rudalics]

 > Sorry, I missed this -- should 0454bfd3313c069ca395f02ab6f377a17ff44965
 > be reverted?

It should be reverted.  Calling `window--before-delete-windows' from
`minimize-window' makes no sense; it should have been called from
`delete-other-windows' instead.  But I am not able to tell what
`window--before-delete-windows' is supposed to do.  And it apparently
causes Bug#47461 and I doubt that this is due to `minimize-window'.

martin

bug#23621: PATCH (25.1.50; Buffer in >1 window; winner-undo recover window point)

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> Sorry, I missed this -- should 0454bfd3313c069ca395f02ab6f377a17ff44965
>> be reverted?
>
> It should be reverted.

OK; now done.  It wasn't a clean revert, since things had changed in
this area, so I had to do it manually, and hopefully I didn't mess that
up...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#23621: 25.1.50; Buffer in >1 window; winner-undo recover window point

[Michael Heerdegen]

Tino Calancha <f92capac@gmail.com> writes:

> Subject: [PATCH] Editing same buffer in >1 window; winner-undo enhancement

An alternative approach maybe: the original value of point is correctly
saved in the saved window-configuration.  That you get the same value of
point for winner-undo in this case is a result of winner changing point
in all buffers to the latest one (of saved buffer window points), AFAIU.

Which is ok in most cases, but not when using multiple windows for one
buffer: then you want to have the original window point.

So, would it be appropriate to change the algorithm to skip changing
point to be different from that of the saved window configuration when
the displayed buffer is visible in multiple windows in the selected
frame?

Michael.