bug#7350: 24.0.50; make vc-deduce-backend smarter

[Bob Rogers <rogers@rgrjr.dyndns.org>]

   From: Stefan Monnier <monnier@iro.umontreal.ca>
   Date: Mon, 15 Nov 2010 11:05:11 -0500

   > Hmm.  TRT would be to figure this out in the "interactive" form, so that
   > repeat-complex-command works, but that would change the args to vc-diff.
   > How big a change are you willing to contemplate?

   A largish patch is not a problem in and of itself (except for copyright
   reasons, but once you've signed the paperwork it's a non-issue).  So the
   only reason to reject such a change would be if it made the code
   conceptually more tricky/complex.  From the sound of it, it would make
   it actually more regular, more in line with the usual way commands work
   in Emacs, so it seems OK.

The problem, of course, is knowing when to stop.  ;-}  vc-diff calls
vc-version-diff, which should also be changed to accept a fileset, but
that's tricky because it's called via call-interactively . . .

   Anyway, I've appended a work-in-progress patch, FYI.  It depends on
refactoring vc-deduce-fileset, which in turn depends on the assumption
that vc-parent-buffer must be registered.  Comments welcomed.

   Of course, there is also the risk of introducing bugs/incompatibilities,
   so try and make extra sure you take into account all callers.

	   Stefan

Grepping found only one caller; vc-log-edit passes it as the
log-edit-diff-function.  But this really ought to use the passed
fileset, and not go through the command again, true?

					-- Bob

------------------------------------------------------------------------
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 665dafb..0681ed3 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -927,31 +927,20 @@ Within directories, only files already under version control are noticed."
 (declare-function vc-dir-current-file "vc-dir" ())
 (declare-function vc-dir-deduce-fileset "vc-dir" (&optional state-model-only-files))
 
-(defun vc-deduce-fileset (&optional observer allow-unregistered
-				    state-model-only-files)
-  "Deduce a set of files and a backend to which to apply an operation.
+(defun vc-deduce-fileset-internal (&optional nonviolent-p state-model-only-files)
+  "Deduce a set of registered files and a backend for an operation.
 
 Return (BACKEND FILESET FILESET-ONLY-FILES STATE CHECKOUT-MODEL).
-If we're in VC-dir mode, the fileset is the list of marked files.
-Otherwise, if we're looking at a buffer visiting a version-controlled file,
-the fileset is a singleton containing this file.
-If none of these conditions is met, but ALLOW_UNREGISTERED is on and the
-visited file is not registered, return a singleton fileset containing it.
-Otherwise, throw an error.
+See vc-deduce-fileset for details; we do just the first part of the search,
+looking for registered files, returning nil if nothing found.
 
-STATE-MODEL-ONLY-FILES if non-nil, means that the caller needs
-the FILESET-ONLY-FILES STATE and MODEL info.  Otherwise, that
-part may be skipped.
-BEWARE: this function may change the
-current buffer."
-  ;; FIXME: OBSERVER is unused.  The name is not intuitive and is not
-  ;; documented.  It's set to t when called from diff and print-log.
+BEWARE: this function may change the current buffer."
   (let (backend)
     (cond
      ((derived-mode-p 'vc-dir-mode)
       (vc-dir-deduce-fileset state-model-only-files))
      ((derived-mode-p 'dired-mode)
-      (if observer
+      (if nonviolent-p
 	  (vc-dired-deduce-fileset)
 	(error "State changing VC operations not supported in `dired-mode'")))
      ((setq backend (vc-backend buffer-file-name))
@@ -964,11 +953,37 @@ current buffer."
      ((and (buffer-live-p vc-parent-buffer)
            ;; FIXME: Why this test?  --Stef
            (or (buffer-file-name vc-parent-buffer)
-				(with-current-buffer vc-parent-buffer
-				  (derived-mode-p 'vc-dir-mode))))
+	       (with-current-buffer vc-parent-buffer
+		 (derived-mode-p 'vc-dir-mode))))
+      ;; Note that vc-parent-buffer must be registered.
       (progn                  ;FIXME: Why not `with-current-buffer'? --Stef.
 	(set-buffer vc-parent-buffer)
-	(vc-deduce-fileset observer allow-unregistered state-model-only-files)))
+	(vc-deduce-fileset-internal nonviolent-p state-model-only-files))))))
+
+(defun vc-deduce-fileset (&optional nonviolent-p allow-unregistered
+				    state-model-only-files)
+  "Deduce a set of files and a backend to which to apply an operation.
+
+Return (BACKEND FILESET FILESET-ONLY-FILES STATE CHECKOUT-MODEL).
+If we're in VC-dir mode, the fileset is the list of marked files.
+Otherwise, if we're in dired-mode, use current/marked files.
+Otherwise, if we're looking at a buffer visiting a version-controlled file,
+the fileset is a singleton containing this file.
+Otherwise, if we're in a VC buffer that has a parent, try again in the parent.
+If none of these conditions is met, but ALLOW-UNREGISTERED is true and the
+visited file is not registered, return a singleton fileset containing it.
+Otherwise, throw an error.
+
+NONVIOLENT-P means that the fileset will be used for a non-state-changing
+operation, such as vc-log or vc-diff.
+
+STATE-MODEL-ONLY-FILES if non-nil, means that the caller needs
+the FILESET-ONLY-FILES STATE and MODEL info.  Otherwise, that
+part may be skipped.
+BEWARE: this function may change the
+current buffer."
+  (cond
+     ((vc-deduce-fileset-internal nonviolent-p state-model-only-files))
      ((not buffer-file-name)
        (error "Buffer %s is not associated with a file" (buffer-name)))
      ((and allow-unregistered (not (vc-registered buffer-file-name)))
@@ -980,7 +995,7 @@ current buffer."
 		nil)
 	(list (vc-backend-for-registration (buffer-file-name))
 	      (list buffer-file-name))))
-     (t (error "No fileset is available here")))))
+     (t (error "No fileset is available here"))))
 
 (defun vc-dired-deduce-fileset ()
   (let ((backend (vc-responsible-backend default-directory)))
@@ -1650,7 +1665,7 @@ returns t if the buffer had changes, nil otherwise."
 		    (called-interactively-p 'interactive)))
 
 ;;;###autoload
-(defun vc-diff (historic &optional not-urgent)
+(defun vc-diff (historic &optional fileset not-urgent)
   "Display diffs between file revisions.
 Normally this compares the currently selected fileset with their
 working revisions.  With a prefix argument HISTORIC, it reads two revision
@@ -1658,11 +1673,24 @@ designators specifying which revisions to compare.
 
 The optional argument NOT-URGENT non-nil means it is ok to say no to
 saving the buffer."
-  (interactive (list current-prefix-arg t))
+  (interactive
+    (list current-prefix-arg
+	  (if current-prefix-arg
+	      nil
+	      (or (vc-deduce-fileset-internal t)
+		  (let ((file (read-file-name "VC diff file: "
+					      default-directory
+					      default-directory)))
+		    (list (or (vc-responsible-backend file)
+			      (error "%S is not under version control." file))
+			  (list (file-truename file))))))
+	  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
+    (when buffer-file-name
+      ;; [there should be a vc-fileset-sync instead.  -- rgr, 16-Nov-10.]
+      (vc-buffer-sync not-urgent))
+    (vc-diff-internal t fileset nil nil
 		      (called-interactively-p 'interactive))))
 
 ;;;###autoload