GNU bug report logs - #41779
Fall back between vc-diff and diff-backup

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 41779 in the body.
You can then email your comments to 41779 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: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Fall back between vc-diff and diff-backup
Date: Wed, 10 Jun 2020 02:35:48 +0300

[Message part 1 (text/plain, inline)]

Tags: patch

>> Which reminds me -- why doesn't `C-x v =' work on normal files that have
>> backups?  I think that would be pretty nice, natural and useful...
>
> I have the same problem of sometimes typing `C-x v =' instead of `M-='
> (dired-backup-diff) on backuped files in dired, and vice versa - `M-='
> instead of `C-x v =' on VC files.

Now this annoyance finally got me - using these keys throws an error
most of the time: when files are not yet registered in vc I use `M-=',
but after registering them in vc, I continue typing the same key `M-='
and get that annoying error.  The situation with `C-x v =' is not better:
often after forgetting that a file is not yet under vc control, typing
`C-x v =' raises the same error.  Therefore, this patch to the rescue:

[vc-diff-backup.patch (text/x-diff, inline)]

diff --git a/lisp/vc/diff.el b/lisp/vc/diff.el
index 9e7e771963..c94c442af8 100644
--- a/lisp/vc/diff.el
+++ b/lisp/vc/diff.el
@@ -230,10 +233,16 @@ diff-backup
     (if (backup-file-name-p file)
 	(setq bak file
 	      ori (file-name-sans-versions file))
-      (setq bak (or (diff-latest-backup-file file)
-		    (error "No backup found for %s" file))
+      (setq bak (diff-latest-backup-file file)
 	    ori file))
-    (diff bak ori switches)))
+    (if bak
+        (diff bak ori switches)
+      ;; Fall back to vc-diff
+      (if (vc-backend file)
+          (let ((vc-diff-switches switches))
+            (vc-diff-internal
+             t (list (vc-backend file) (list file)) nil nil t))
+        (error "No backup found for %s" file)))))
 
 ;;;###autoload
 (defun diff-latest-backup-file (fn)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c640ba0420..a0363bd774 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1888,11 +1890,15 @@ vc-diff
 The optional argument NOT-URGENT non-nil means it is ok to say no to
 saving the buffer."
   (interactive (list current-prefix-arg t))
-  (if historic
-      (call-interactively 'vc-version-diff)
-    (when buffer-file-name (vc-buffer-sync not-urgent))
-    (vc-diff-internal t (vc-deduce-fileset t) nil nil
-		      (called-interactively-p 'interactive))))
+  (if (vc-deduce-backend)
+      (if historic
+          (call-interactively 'vc-version-diff)
+        (when buffer-file-name (vc-buffer-sync not-urgent))
+        (vc-diff-internal t (vc-deduce-fileset t) nil nil
+		          (called-interactively-p 'interactive)))
+    ;; Fall back to non-vc diff-backup
+    (diff-backup (if (derived-mode-p 'dired-mode) (dired-get-filename) buffer-file-name)
+                 vc-diff-switches)))
 
 ;;;###autoload
 (defun vc-diff-mergebase (_files rev1 rev2)

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Wed, 10 Jun 2020 22:05:35 +0300

On 10.06.2020 02:35, Juri Linkov wrote:
>   Therefore, this patch to the rescue:

Considering that 'M-=' is not a default binding, and diff-backup doesn't 
have one, how about creating a new command? You could just as well bind 
it to 'M-='.

The command could be called 'diff-dwim'. Or 'diff-vc-or-backup'.

Regarding the proposed patch, I'm somewhat uneasy making 'vc-diff' (the 
command which purpose is to compare edits against the "saved" snapshot 
in version control) diff against the backup (lifetime of which is very 
unclear to me). Or vice versa.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Thu, 11 Jun 2020 00:53:56 +0300

> Considering that 'M-=' is not a default binding, and diff-backup doesn't
> have one, how about creating a new command? You could just as well bind 
> it to 'M-='.
>
> The command could be called 'diff-dwim'. Or 'diff-vc-or-backup'.
>
> Regarding the proposed patch, I'm somewhat uneasy making 'vc-diff' (the
> command which purpose is to compare edits against the "saved" snapshot 
> in version control) diff against the backup (lifetime of which is very
> unclear to me). Or vice versa.

Why do you think that throwing an error is more useful than doing
some nice fallback?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Thu, 11 Jun 2020 01:18:23 +0300

On 11.06.2020 00:53, Juri Linkov wrote:
> Why do you think that throwing an error is more useful than doing
> some nice fallback?

It's... imprecise? Like, if I'm using the 'C-x v' prefix, I expect to 
work against a VC repository. Having a command like that succeed despite 
the lack of said repository could give a wrong impression to the user.

Not a big problem, but the cost of implementing a different solution 
doesn't seem to be high either.

Admittedly, I use diff-backup very rarely (though it does happen from 
time to time), so I might not understand your usage scenarios well.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Thu, 11 Jun 2020 02:17:02 +0300

>> Why do you think that throwing an error is more useful than doing
>> some nice fallback?
>
> It's... imprecise? Like, if I'm using the 'C-x v' prefix, I expect to work
> against a VC repository. Having a command like that succeed despite the
> lack of said repository could give a wrong impression to the user.
>
> Not a big problem, but the cost of implementing a different solution
> doesn't seem to be high either.
>
> Admittedly, I use diff-backup very rarely (though it does happen from time
> to time), so I might not understand your usage scenarios well.

So for the normal usage, users won't notice this additional feature,
and in case of the user error it comes to the rescue.

But I don't insist on this feature, I can handle this in my init file.
I proposed it as a patch only because Lars mentioned the same problem.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Thu, 11 Jun 2020 02:26:05 +0300

On 11.06.2020 02:17, Juri Linkov wrote:
> So for the normal usage, users won't notice this additional feature,
> and in case of the user error it comes to the rescue.

In a way that is likely to make the user miss the fact that there was a 
misconception.

> But I don't insist on this feature, I can handle this in my init file.
> I proposed it as a patch only because Lars mentioned the same problem.

I think my suggestion is not significantly worse or more awkward to use.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 41779 <at> debbugs.gnu.org
Subject: Re: bug#41779: Fall back between vc-diff and diff-backup
Date: Thu, 25 Jun 2020 02:32:32 +0300

tags 41779 wontfix
close 41779 28.0.50
quit

>> But I don't insist on this feature, I can handle this in my init file.
>> I proposed it as a patch only because Lars mentioned the same problem.
>
> I think my suggestion is not significantly worse or more awkward to use.

It seems my request was too specific, so I retract it.
It's easy to achieve this fallback in the init file with:

(advice-add 'diff-backup :around
            (lambda (orig-fun file &optional switches)
              (condition-case err
                  (funcall orig-fun file switches)
                (error
                 ;; Fall back to vc-diff
                 (if (vc-backend file)
                     (let ((vc-diff-switches switches))
                       (require 'vc)
                       (vc-diff-internal
                        t (list (vc-backend file) (list file)) nil nil t))
                   (error (cadr err))))))
            '((name . diff-backup-fallback-to-vc-diff)))

(advice-add 'vc-diff :around
            (lambda (orig-fun &rest args)
              (if (vc-deduce-backend)
                  (apply orig-fun args)
                ;; Fall back to non-vc diff-backup
                (diff-backup (if (derived-mode-p 'dired-mode)
                                 (dired-get-filename)
                               buffer-file-name)
                             vc-diff-switches)))
            '((name . vc-diff-fallback-to-diff-backup)))

Previous Next

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