Subject: Re: bug#73232: [PATCH] Allow vc-diff to suggest a default revision in vc-dir

From: Dmitry Gutov <[email-masked]>
Date: Sat, 28 Sep 2024 00:55:23 +0300

And here's the diff.

On 28/09/2024 00:49, Dmitry Gutov wrote:
> Hi all,
> 
> On 14/09/2024 04:45, Dmitry Gutov wrote:
>>
>> I can see where you're coming from though -- that default isn't very 
>> useful, one might as well not press C-u.
>>
>> Maybe we should switch to suggesting the previous revision in the 
>> prompt even when file has changes?
> 
> Here's what seems to me an overall improvement, based on the original 
> change. And more consistent as well.
> 
> * No special case for when FIRST is a directory OR it's not up-to-date.
> * Make REV1-DEFAULT a list value.
> * In 'vc-root-version-diff', don't try calling 'vc-deduce-fileset' and 
> construct a (BACKEND DEFAULT-DIR) fileset right away.
> 
> As a result, 'C-u C-x v d' consistently provides completion and diff 
> relating to the whole repository, not for files as point (if any). 
> Previously, it used the revision that last touched the corresponding 
> file, or nil, if the file was untracked (e.g. in Dired).
> 
> Further, don't offer the working revision as REV1-DEFAULT. Except for 
> historical reasons and some idea of consistency, I can't see a scenario 
> where that would be useful, which would not be covered by calling 'C-x v 
> d' without a prefix. Someone please correct me here.
> 
> And combined with Spencer's patch from https://debbugs.gnu.org/62940#46, 
> we get this:
> 
> * First default is HEAD^ (the last revision before the latest).
> * Second default is @{upstream}.
> * Then the elements from vc-revision-history.
> 
> WDYT?
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 597a1622f5a..8f3200c1a39 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2074,20 +2074,15 @@ vc-diff-build-argument-list-internal
      ;; filesets, but not yet.
      ((/= (length files) 1)
       nil)
-     ;; if it's a directory, don't supply any revision default
-     ((file-directory-p first)
-      nil)
-     ;; if the file is not up-to-date, use working revision as older revision
-     ((not (vc-up-to-date-p first))
-      (setq rev1-default (vc-working-revision first)))
      ;; if the file is not locked, use last revision and current source as defaults
      (t
-      (setq rev1-default (ignore-errors ;If `previous-revision' doesn't work.
-                           (vc-call-backend backend 'previous-revision first
-                                            (vc-working-revision first))))
-      (when (string= rev1-default "") (setq rev1-default nil))))
+      (push (ignore-errors         ;If `previous-revision' doesn't work.
+              (vc-call-backend backend 'previous-revision first
+                               (vc-working-revision first backend)))
+            rev1-default)
+      (when (member (car rev1-default) '("" nil)) (setq rev1-default nil))))
     ;; construct argument list
-    (let* ((rev1-prompt (format-prompt "Older revision" rev1-default))
+    (let* ((rev1-prompt (format-prompt "Older revision" (car rev1-default)))
            (rev2-prompt (format-prompt "Newer revision"
                                        ;; (or rev2-default
                                        "current source"))
@@ -2119,9 +2114,8 @@ vc-root-version-diff
   "Report diffs between REV1 and REV2 revisions of the whole tree."
   (interactive
    (vc-diff-build-argument-list-internal
-    (or (ignore-errors (vc-deduce-fileset t))
-        (let ((backend (or (vc-deduce-backend) (vc-responsible-backend default-directory))))
-          (list backend (list (vc-call-backend backend 'root default-directory)))))))
+    (let ((backend (or (vc-deduce-backend) (vc-responsible-backend default-directory))))
+      (list backend (list (vc-call-backend backend 'root default-directory))))))
   ;; This is a mix of `vc-root-diff' and `vc-version-diff'
   (when (and (not rev1) rev2)
     (error "Not a valid revision range"))