Subject: Re: bug#24089: 25.1; Dired: diff error on = <RET> if the file has a backup inside .~/

From: Eli Zaretskii <[email-masked]>
Date: Sun, 31 Jul 2016 18:08:40 +0300

> From: Jorge Peixoto de Morais Neto <[email-masked]>
> Cc: [email-masked]
> Date: Fri, 29 Jul 2016 13:29:23 -0300
> 
> On Fri, 2016-07-29 at 17:43 +0300, Eli Zaretskii wrote:
> > I'm not sure why you expect that case to work.  How can Emacs
> > reverse-engineer backup-directory-alist, to figure out which of its
> > elements was in effect when the original file was backed up?  In your
> > example, there's only one element in the alist, but in general there
> > could be several.
> > 
> > It sounds like in this case the users cannot just press RET, they
> > should actually type a file name.  Am I missing something?
> I understood the intended behavior that way because of the dired-diff
> docstring:
>     If the file at point is a backup file, use its original.
> 
> Perhaps the docstring could be clarified?
> 
> Also dired-diff could be tweaked so that, when it is is invoked on a backup
> file, the default would be empty.  This would avoid the diff error on = <RET>.
>  I am not sure though.
> 
> Another possibility (which can be combined with the previous) is to tweak
> dired-diff to reverse-engineer backup-directory-alist at least for a few
> important easy particular cases, such as the following two:
>    1. (("." . ".~"))
>    2. (("." . "/ABSOLUTE/PATH"))
> and possibly others.

How about the alternative patch below?  It fixes the doc string, and
makes the prompt be without the default in the second case.  Is that
good enough, in your opinion?

--- lisp/dired-aux.el~0	2016-05-08 07:05:52.000000000 +0300
+++ lisp/dired-aux.el	2016-07-31 17:50:35.792144200 +0300
@@ -55,19 +55,23 @@
 
 ;;;###autoload
 (defun dired-diff (file &optional switches)
-  "Compare file at point with file FILE using `diff'.
-If called interactively, prompt for FILE.  If the file at point
-has a backup file, use that as the default.  If the file at point
-is a backup file, use its original.  If the mark is active
-in Transient Mark mode, use the file at the mark as the default.
-\(That's the mark set by \\[set-mark-command], not by Dired's
-\\[dired-mark] command.)
+  "Compare file at point with FILE using `diff'.
+If called interactively, prompt for FILE.
+If the mark is active in Transient Mark mode, use the file at the mark
+as the default for FILE.  (That's the mark set by \\[set-mark-command],
+not by Dired's \\[dired-mark] command.)
+If the file at point has a backup file, use that as the default FILE.
+If the file at point is a backup file, use its original, if that exists
+and can be found.  Note that customizations of `backup-directory-alist'
+and `make-backup-file-name-function' change where this function searches
+for the backup file, and affect its ability to find the original of a
+backup file.
 
-FILE is the first file given to `diff'.  The file at point
-is the second file given to `diff'.
+FILE is the first argument given to the `diff' function.  The file at
+point is the second argument given to `diff'.
 
 With prefix arg, prompt for second argument SWITCHES, which is
-the string of command switches for the third argument of `diff'."
+the string of command switches used as the third argument of `diff'."
   (interactive
    (let* ((current (dired-get-filename t))
 	  ;; Get the latest existing backup file or its original.
@@ -78,8 +82,20 @@
 	  (file-at-mark (if (and transient-mark-mode mark-active)
 			    (save-excursion (goto-char (mark t))
 					    (dired-get-filename t t))))
+          (separate-dir (and oldf
+                             (not (equal (file-name-directory oldf)
+                                         (dired-current-directory)))))
 	  (default-file (or file-at-mark
-			    (and oldf (file-name-nondirectory oldf))))
+                            ;; If the file with which to compare
+                            ;; doesn't exist, or we cannot intuit it,
+                            ;; we forget that name and don't show it
+                            ;; as the default, as an indication to the
+                            ;; user that she should type the file
+                            ;; name.
+			    (and (if (and oldf (file-readable-p oldf)) oldf)
+                                 (if separate-dir
+                                     oldf
+                                   (file-name-nondirectory oldf)))))
 	  ;; Use it as default if it's not the same as the current file,
 	  ;; and the target dir is current or there is a default file.
 	  (default (if (and (not (equal default-file current))
@@ -88,7 +104,9 @@
 				default-file))
 		       default-file))
 	  (target-dir (if default
-			  (dired-current-directory)
+                          (if separate-dir
+                              (file-name-directory default)
+                            (dired-current-directory))
 			(dired-dwim-target-directory)))
 	  (defaults (dired-dwim-target-defaults (list current) target-dir)))
      (list