GNU bug report logs - #28852
make revert-buffer ('g') in VC diff buffers keep point

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28852 in the body.
You can then email your comments to 28852 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: bug-gnu-emacs <at> gnu.org
Subject: make revert-buffer ('g') in VC diff buffers keep point
Date: Sun, 15 Oct 2017 21:07:30 +0200

At the moment, when you hit 'g' in a *vc-diff* buffer, the cursor is
moved back to (point-min), which can be annoying.  Attached is an
attempt at fixing that (using 'replace-buffer-contents' to keep
markers/point in the same place).

<#part type="text/x-patch" filename="~/Datacar/Code/emacs-devel/0001-Make-revert-buffer-g-keep-point-in-VC-diff-buffers.patch" disposition=attachment>
<#/part>

Message #8 received at 28852 <at> debbugs.gnu.org (full text, mbox):

From: charles <at> aurox.ch (Charles A. Roelli)
To: 28852 <at> debbugs.gnu.org
Subject: Re: bug#28852: make revert-buffer ('g') in VC diff buffers keep point
Date: Tue, 17 Oct 2017 19:58:00 +0200

> Date: Sun, 15 Oct 2017 21:07:30 +0200
> From: charles <at> aurox.ch (Charles A. Roelli)
> 
> At the moment, when you hit 'g' in a *vc-diff* buffer, the cursor is
> moved back to (point-min), which can be annoying.  Attached is an
> attempt at fixing that (using 'replace-buffer-contents' to keep
> markers/point in the same place).

Hm, I'll try again:

From 48e3febfee28276b3eb7d9af58342c70d2d798f9 Mon Sep 17 00:00:00 2001
From: "Charles A. Roelli" <charles <at> aurox.ch>
Date: Sun, 15 Oct 2017 20:58:01 +0200
Subject: [PATCH] Make revert-buffer ('g') keep point in VC diff buffers
 (Bug#28852)

* lisp/vc/vc.el (vc-diff-restore-buffer): New function.
(vc-diff-finish): Update its calling convention to include an
optional 'oldbuf' parameter, and handle it.
(vc-diff-internal): Pass a clone of the incumbent vc-diff
buffer to 'vc-diff-finish'.
---
 lisp/vc/vc.el | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index b80f0e6..57d5a50 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1654,7 +1654,20 @@ vc-diff-switches-list
   (declare (obsolete vc-switches "22.1"))
   `(vc-switches ',backend 'diff))
 
-(defun vc-diff-finish (buffer messages)
+(defun vc-diff-restore-buffer (original new)
+  "Restore point in buffer NEW to where it was in ORIGINAL.
+
+This function works by updating buffer ORIGINAL with the contents
+of NEW (without destroying existing markers), swapping their text
+objects, and finally killing buffer ORIGINAL."
+  (with-current-buffer original
+    (let ((inhibit-read-only t))
+      (replace-buffer-contents new)))
+  (with-current-buffer new
+    (buffer-swap-text original))
+  (kill-buffer original))
+
+(defun vc-diff-finish (buffer messages &optional oldbuf)
   ;; The empty sync output case has already been handled, so the only
   ;; possibility of an empty output is for an async process.
   (when (buffer-live-p buffer)
@@ -1666,7 +1679,13 @@ vc-diff-finish
 	       (insert (cdr messages) ".\n")
 	       (message "%s" (cdr messages))))
 	(diff-setup-whitespace)
-	(goto-char (point-min))
+        ;; `oldbuf' is the buffer that used to show this diff.  Make
+        ;; sure that we restore point in it if it's given.
+	(if oldbuf
+            (progn
+              (vc-diff-restore-buffer oldbuf buffer)
+              (diff-mode))
+          (goto-char (point-min)))
 	(when window
 	  (shrink-window-if-larger-than-buffer window)))
       (when (and messages (not emptyp))
@@ -1692,7 +1711,12 @@ vc-diff-internal
 	 ;; but the only way to set it for each file included would
 	 ;; be to call the back end separately for each file.
 	 (coding-system-for-read
-	  (if files (vc-coding-system-for-diff (car files)) 'undecided)))
+	  (if files (vc-coding-system-for-diff (car files)) 'undecided))
+         (orig-diff-buffer-clone
+          (if (and (get-buffer buffer) revert-buffer-in-progress-p)
+              (with-current-buffer buffer
+                (clone-buffer
+                 (generate-new-buffer-name " *vc-diff-clone*") nil)))))
     ;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
     ;; EOLs, which will look ugly if (car files) happens to have Unix
     ;; EOLs.
@@ -1752,7 +1776,8 @@ vc-diff-internal
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
-        (vc-run-delayed (vc-diff-finish buf (when verbose messages))))
+        (vc-run-delayed (vc-diff-finish buf (when verbose messages)
+                                        orig-diff-buffer-clone)))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))
-- 
2.9.4

Message #15 received at 28852 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28852 <at> debbugs.gnu.org
Subject: Re: bug#28852: make revert-buffer ('g') in VC diff buffers keep point
Date: Mon, 24 Jun 2019 19:40:57 +0200

charles <at> aurox.ch (Charles A. Roelli) writes:

>> At the moment, when you hit 'g' in a *vc-diff* buffer, the cursor is
>> moved back to (point-min), which can be annoying.

Yes, I agree.

> +(defun vc-diff-restore-buffer (original new)
> +  "Restore point in buffer NEW to where it was in ORIGINAL.
> +
> +This function works by updating buffer ORIGINAL with the contents
> +of NEW (without destroying existing markers), swapping their text
> +objects, and finally killing buffer ORIGINAL."
> +  (with-current-buffer original
> +    (let ((inhibit-read-only t))
> +      (replace-buffer-contents new)))
> +  (with-current-buffer new
> +    (buffer-swap-text original))
> +  (kill-buffer original))

This will restore point to the same point it was before...  but not to
"where it was", necessarily, I think?  That is, if your point is at
character 105, then it'll be on character 105 after this, but since the
contents may (and will) change, what's on character 105 may be totally
different after running the diff -- new changes will appear and others
will disappear.

I remember discussing a "sloppy save excursion" on emacs-devel once:
Save as much data as possible about the old buffer contents, and then
try to move point to a similar context in the new buffer contents, even
if that means moving to a different line.

This is a general problem in many buffers that display generated content
and need to get back to "where it was" after a `g'.

But I can't remember what the conclusion was, and now I can't find the
discussion...  Does anybody remember?

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

Message #22 received at 28852 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: charles <at> aurox.ch (Charles A. Roelli)
Cc: 28852 <at> debbugs.gnu.org
Subject: Re: bug#28852: make revert-buffer ('g') in VC diff buffers keep point
Date: Sat, 29 Jan 2022 16:48:40 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> This will restore point to the same point it was before...  but not to
> "where it was", necessarily, I think? 

I was confused about how these functions worked.  I've respun the patch
for Emacs 29, and it seem to work very well, so I've now pushed it.

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

Message #27 received at 28852 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 28852 <at> debbugs.gnu.org, "Charles A. Roelli" <charles <at> aurox.ch>
Subject: Re: bug#28852: make revert-buffer ('g') in VC diff buffers keep point
Date: Sat, 29 Jan 2022 20:48:36 +0200

>> This will restore point to the same point it was before...  but not to
>> "where it was", necessarily, I think?
>
> I was confused about how these functions worked.  I've respun the patch
> for Emacs 29, and it seem to work very well, so I've now pushed it.

This doesn't work with such customization:

  (add-hook 'diff-mode-hook 'rename-uniquely)

because this change assumes that the current buffer is always "*vc-diff*"
whereas the reverted buffer can be "*vc-diff*<2>".

Isn't the reverted buffer always current during revert-buffer command?
If this assumption is correct, how about his fix?

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0096a5fcb3..1671f2f860 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1798,10 +1798,9 @@ vc-diff-internal
 	 (coding-system-for-read
 	  (if files (vc-coding-system-for-diff (car files)) 'undecided))
          (orig-diff-buffer-clone
-          (if (and (get-buffer buffer) revert-buffer-in-progress-p)
-              (with-current-buffer buffer
-                (clone-buffer
-                 (generate-new-buffer-name " *vc-diff-clone*") nil)))))
+          (if revert-buffer-in-progress-p
+              (clone-buffer
+               (generate-new-buffer-name " *vc-diff-clone*") nil))))
     ;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
     ;; EOLs, which will look ugly if (car files) happens to have Unix
     ;; EOLs.
-- 

Message #30 received at 28852 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 28852 <at> debbugs.gnu.org, "Charles A. Roelli" <charles <at> aurox.ch>
Subject: Re: bug#28852: make revert-buffer ('g') in VC diff buffers keep point
Date: Sun, 30 Jan 2022 16:55:10 +0100

Juri Linkov <juri <at> linkov.net> writes:

> Isn't the reverted buffer always current during revert-buffer command?

Yes, I think so.

> If this assumption is correct, how about his fix?

Makes sense to me.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.