bug#33319: Support revisions in diff-goto-source

[Juri Linkov <juri@linkov.net>]

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

>> +*** In the diff buffer created by a version control system, the prefix
>> +arg of diff-goto-source means it jumps to the old revision of the file
>                            ^^^^^^^^^^^^^^^^^
> "... means jump to ..." is a better wording, I think.
>
> Also, it is best to provide a header for the description of the
> change, so that it could be meaningfully folded by Outline mode.

Fixed in a new patch.

> And I think we want to update the manual as well.

Updated.

>> +(defvar diff-vc-revisions nil
>> +  "The VC revisions compared in the current Diff buffer, if any.")
>> +
>>  (defvar diff-outline-regexp
>>    "\\([*+][*+][*+] [^0-9]\\|@@ ...\\|\\*\\*\\* [0-9].\\|--- [0-9]..\\)")
>
> Which VCSes does this support?  I'm not sure all of them produce such
> markers.  If this supports only some, we should document that and make
> sure the code does something reasonable when the revision is not
> found.

It should work in all VCSes that support the VC interface function
'find-revision' defined in the comments of vc.el:

;; * find-revision (file rev buffer)
;;
;;   Fetch revision REV of file FILE and put it into BUFFER.
;;   If REV is the empty string, fetch the head of the trunk.
;;   The implementation should pass the value of vc-checkout-switches
;;   to the backend command.

BTW, its implementation in vc-find-revision currently has
a major shortcoming: it always saves the retrieved revision
to the file.  I spend too much time cleaning after this command
all old files that it creates.  Thus I propose a new option
`vc-find-revision-no-save' to not save the buffer it creates
to the file.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: diff-goto-source.2.patch --]
[-- Type: text/x-diff, Size: 6699 bytes --]

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 6c68075ae4..523734ec1f 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1507,7 +1507,11 @@ Diff Mode
 @item C-c C-c
 @findex diff-goto-source
 Go to the source file and line corresponding to this hunk
-(@code{diff-goto-source}).
+(@code{diff-goto-source}).  With a prefix argument of @kbd{C-u},
+go to the old source file.  If the source file is under version
+control (@pxref{Version Control}), with a prefix argument of
+@kbd{C-u}, go to the old revision of the file (@pxref{Old Revisions})
+when point is on old line, or otherwise to the new revision.
 
 @item C-c C-e
 @findex diff-ediff-patch
diff --git a/etc/NEWS b/etc/NEWS
index 29bbde9395..cea600c430 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -334,6 +334,9 @@ still be used if it exists.)  Set the variable to nil to get the
 previous behavior of always creating a buffer that visits a ChangeLog
 file.
 
+*** New customizable variable 'vc-find-revision-no-save'.
+With non-nil, 'vc-find-revision' doesn't write the created buffer to file.
+
 *** New customizable variable 'vc-git-grep-template'.
 This new variable allows customizing the default arguments passed to
 git-grep when 'vc-git-grep' is used.
@@ -358,6 +361,10 @@ To disable it, set the new defcustom 'diff-font-lock-refine' to nil.
 *** File headers can be shortened, mimicking Magit's diff format.
 To enable it, set the new defcustom 'diff-font-lock-prettify to t.
 
+*** Prefix arg of 'diff-goto-source' means jump to the old revision
+of the file under version control if point is on an old changed line,
+or to the new revision of the file otherwise.
+
 ** Browse-url
 
 *** The function 'browse-url-emacs' can now visit a URL in selected window.
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index cf52368508..aef16e2e67 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -104,6 +105,9 @@ diff-font-lock-prettify
 (defvar diff-vc-backend nil
   "The VC backend that created the current Diff buffer, if any.")
 
+(defvar diff-vc-revisions nil
+  "The VC revisions compared in the current Diff buffer, if any.")
+
 (defvar diff-outline-regexp
   "\\([*+][*+][*+] [^0-9]\\|@@ ...\\|\\*\\*\\* [0-9].\\|--- [0-9]..\\)")
 
@@ -1736,7 +1740,12 @@ diff-find-source-location
 		       (match-string 1)))))
 	   (file (or (diff-find-file-name other noprompt)
                      (error "Can't find the file")))
-	   (buf (find-file-noselect file)))
+	   (revision (and other diff-vc-backend
+                          (nth (if reverse 1 0) diff-vc-revisions)))
+	   (buf (if revision
+                    (let ((vc-find-revision-no-save t))
+                      (vc-find-revision file revision diff-vc-backend))
+                  (find-file-noselect file))))
       ;; Update the user preference if he so wished.
       (when (> (prefix-numeric-value other-file) 8)
 	(setq diff-jump-to-old-file other))
@@ -1862,7 +1871,11 @@ diff-goto-source
 `diff-jump-to-old-file' (or its opposite if the OTHER-FILE prefix arg
 is given) determines whether to jump to the old or the new file.
 If the prefix arg is bigger than 8 (for example with \\[universal-argument] \\[universal-argument])
-then `diff-jump-to-old-file' is also set, for the next invocations."
+then `diff-jump-to-old-file' is also set, for the next invocations.
+
+Under version control, the OTHER-FILE prefix arg means jump to the old
+revision of the file if point is on an old changed line, or to the new
+revision of the file otherwise."
   (interactive (list current-prefix-arg last-input-event))
   ;; When pointing at a removal line, we probably want to jump to
   ;; the old location, and else to the new (i.e. as if reverting).
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index dcfbf26e86..7c4c288b66 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -871,6 +871,12 @@ vc-comment-alist
 		       (string :tag "Comment End")))
   :group 'vc)
 
+(defcustom vc-find-revision-no-save nil
+  "If non-nil, `vc-find-revision' doesn't write the created buffer to file."
+  :type 'boolean
+  :group 'vc
+  :version "27.1")
+
 \f
 ;; File property caching
 
@@ -1728,6 +1735,7 @@ vc-diff-internal
     (set-buffer buffer)
     (diff-mode)
     (set (make-local-variable 'diff-vc-backend) (car vc-fileset))
+    (set (make-local-variable 'diff-vc-revisions) (list rev1 rev2))
     (set (make-local-variable 'revert-buffer-function)
 	 (lambda (_ignore-auto _noconfirm)
            (vc-diff-internal async vc-fileset rev1 rev2 verbose)))
@@ -1951,6 +1959,8 @@ vc-revision-other-window
 (defun vc-find-revision (file revision &optional backend)
   "Read REVISION of FILE into a buffer and return the buffer.
 Use BACKEND as the VC backend if specified."
+  (if vc-find-revision-no-save
+      (vc-find-revision-no-save file revision backend)
   (let ((automatic-backup (vc-version-backup-file-name file revision))
 	(filebuf (or (get-file-buffer file) (current-buffer)))
         (filename (vc-version-backup-file-name file revision 'manual)))
@@ -1981,6 +1991,38 @@ vc-find-revision
 	;; Set the parent buffer so that things like
 	;; C-x v g, C-x v l, ... etc work.
 	(set (make-local-variable 'vc-parent-buffer) filebuf))
+      result-buf))))
+
+(defun vc-find-revision-no-save (file revision &optional backend)
+  "Read REVISION of FILE into a buffer and return the buffer.
+Unlike `vc-find-revision', doesn't save the created buffer to file."
+  (let ((filebuf (or (get-file-buffer file) (current-buffer)))
+        (filename (vc-version-backup-file-name file revision 'manual)))
+    (unless (or (get-file-buffer filename)
+                (file-exists-p filename))
+      (with-current-buffer filebuf
+	(let ((failed t))
+	  (unwind-protect
+	      (let ((coding-system-for-read 'no-conversion)
+		    (coding-system-for-write 'no-conversion))
+		(with-current-buffer (create-file-buffer filename)
+                  (setq buffer-file-name filename)
+		  (let ((outbuf (current-buffer)))
+		    (with-current-buffer filebuf
+		      (if backend
+			  (vc-call-backend backend 'find-revision file revision outbuf)
+			(vc-call find-revision file revision outbuf))))
+                  (goto-char (point-min))
+                  (normal-mode)
+	          (set-buffer-modified-p nil)
+                  (setq buffer-read-only t))
+		(setq failed nil))
+	    (when (and failed (get-file-buffer filename))
+	      (kill-buffer (get-file-buffer filename)))))))
+    (let ((result-buf (or (get-file-buffer filename)
+                          (find-file-noselect filename))))
+      (with-current-buffer result-buf
+	(set (make-local-variable 'vc-parent-buffer) filebuf))
       result-buf)))
 
 ;; Header-insertion code