Subject: bug#33650: 27.0.50; Root diffs between revisions

From: Juri Linkov <[email-masked]>
Date: Sun, 09 Dec 2018 01:32:47 +0200

>> +(defun vc-root-version-diff (_files rev1 rev2)
>> +  "Report diffs between revisions of the whole tree in the repository history."
>
> This doc string doesn't describe the arguments, and doesn't tell how
> those arguments are populated in interactive invocations.

I copied the doc string from `vc-version-diff' that doesn't describe its
arguments.  Now I fixed the doc string of `vc-version-diff' as well below.

> I also wonder what would this mean for a VCS such as CVS or RCS, where
> there's no meaning of a "tree revision".  I'm guessing this is why the
> old code deduced files to diff.  See also the description of vc-diff
> in the Emacs manual.

`vc-root-diff' is not applicable to VCS without tree revisions, and
the manual says nothing about this.  Maybe the manual should say this
explicitly for `vc-root-diff' as it says it for `vc-print-root-log'
in (info "(emacs) VC Change Log")

     ‘C-x v L’ (‘vc-print-root-log’) displays a ‘*vc-change-log*’ buffer
  showing the history of the entire version-controlled directory tree
  (RCS, SCCS, CVS, and SRC do not support this feature).

> Finally, we need documentation changes to go with this change of
> user-visible behavior.

This feature is already documented in (info "(emacs) Old Revisions")

  ‘C-x v D’
       Compare the entire working tree to the revision you started from
       (‘vc-root-diff’).  With a prefix argument, prompt for two revisions
       and compare their trees.

It didn't work as documented before properly implemented by the patch.

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index dbbc3e2038..ba410dca0e 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1817,7 +1817,8 @@ vc-diff-build-argument-list-internal
 
 ;;;###autoload
 (defun vc-version-diff (_files rev1 rev2)
-  "Report diffs between revisions of the fileset in the repository history."
+  "Report diffs between revisions of the fileset in the repository history.
+REV1 is an older revision, REV2 is a newer revision."
   (interactive (vc-diff-build-argument-list-internal))
   ;; All that was just so we could do argument completion!
   (when (and (not rev1) rev2)
@@ -1827,6 +1828,29 @@ vc-version-diff
   (vc-diff-internal t (vc-deduce-fileset t) rev1 rev2
 		    (called-interactively-p 'interactive)))
 
+;;;###autoload
+(defun vc-root-version-diff (_files rev1 rev2)
+  "Report diffs between revisions of the whole tree in the repository history.
+REV1 is an older revision, REV2 is a newer revision."
+  (interactive (vc-diff-build-argument-list-internal))
+  ;; This is a mix of `vc-root-diff' and `vc-version-diff'
+  (when (and (not rev1) rev2)
+    (error "Not a valid revision range"))
+  (let ((backend (vc-deduce-backend))
+        (default-directory default-directory)
+        rootdir)
+    (if backend
+	(setq rootdir (vc-call-backend backend 'root default-directory))
+      (setq rootdir (read-directory-name "Directory for VC root-diff: "))
+      (setq backend (vc-responsible-backend rootdir))
+      (if backend
+	  (setq default-directory rootdir)
+	(error "Directory is not version controlled")))
+    (let ((default-directory rootdir))
+      (vc-diff-internal
+       t (list backend (list rootdir)) rev1 rev2
+       (called-interactively-p 'interactive)))))
+
 ;;;###autoload
 (defun vc-diff (&optional historic not-urgent)
   "Display diffs between file revisions.
@@ -1900,10 +1924,8 @@ vc-root-diff
 saving the buffer."
   (interactive (list current-prefix-arg t))
   (if historic
-      ;; FIXME: this does not work right, `vc-version-diff' ends up
-      ;; calling `vc-deduce-fileset' to find the files to diff, and
-      ;; that's not what we want here, we want the diff for the VC root dir.
-      (call-interactively 'vc-version-diff)
+      ;; We want the diff for the VC root dir.
+      (call-interactively 'vc-root-version-diff)
     (when buffer-file-name (vc-buffer-sync not-urgent))
     (let ((backend (vc-deduce-backend))
 	  (default-directory default-directory)