Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <address@hidden> writes:

> As long as it works (UI-wise) similarly to vc-diff, the behavior
> is fine.  I wanted the implementation to rely as much as possible on VC
> and as little as possible on ediff's own revision control code, but that
> is not crucial, if doing it differently lets it share more code with
> ediff-revision.

I think my original patch adhered to this. I factored out an interface
to be able to call ediff-revision without any user
interaction. ediff-revision and vc-ediff now use this new interface
function to invoke the ediff session. The UI behavior is the same as
vc-diff, with the exception of ediff'ing multiple selected files in
vc-dir, see comment regarding the FIXME below.

> Can you submit a patch according to these guidelines?

I cleaned up the patch and also updated Changelogs, NEWS and manual.

There is one FIXME in the vc-ediff function which is regarding multiple
files selected in vc-dir. vc-diff just shows all the diffs, but vc-ediff
would have to invoke n ediff session for n files selected. For now I
only support selecting 1 file, otherwise it issues an error.

Christoph


=== modified file 'doc/emacs/ChangeLog'
--- doc/emacs/ChangeLog 2011-03-12 19:19:47 +0000
+++ doc/emacs/ChangeLog 2011-03-22 04:27:16 +0000
@@ -1,3 +1,8 @@
+2011-03-22  Christoph Scholtes  <address@hidden>
+
+       * maintaining.texi (Old Revisions): Add paragraph on new function
+       vc-ediff.
+
 2011-03-12  Eli Zaretskii  <address@hidden>
 
        * msdog.texi (Windows HOME): Fix the wording to clarify how Emacs sets

=== modified file 'doc/emacs/maintaining.texi'
--- doc/emacs/maintaining.texi  2011-03-01 04:14:00 +0000
+++ doc/emacs/maintaining.texi  2011-03-22 04:31:50 +0000
@@ -744,6 +744,15 @@
 buffer, these commands generate a diff of all registered files in the
 current directory and its subdirectories.
 
address@hidden vc-ediff
+The function @code{vc-ediff} works similar to @code{vc-diff} and
+provides a way to visually compare two revisions of a file an Ediff
+session, @pxref{Top, Ediff, ediff, The Ediff Manual}.  It compares the
+file associated with the current buffer with the last repository
+revision.  To compare two arbitrary revisions of the current file,
+call @code{vc-ediff} with a prefix argument.  This prompts for two
+revision IDs using the minibuffer.
+
 @vindex vc-diff-switches
 @vindex vc-rcs-diff-switches
   @kbd{C-x v =} works by running a variant of the @code{diff} utility

=== modified file 'etc/ChangeLog'
--- etc/ChangeLog       2011-03-19 03:58:45 +0000
+++ etc/ChangeLog       2011-03-22 02:34:36 +0000
@@ -1,3 +1,7 @@
+2011-03-22  Christoph Scholtes  <address@hidden>
+
+       * NEWS: Document new function `vc-ediff'.
+
 2011-03-16  Juanma Barranquero  <address@hidden>
 
        * NEWS: Document warning about _emacs.

=== modified file 'etc/NEWS'
--- etc/NEWS    2011-03-21 16:34:16 +0000
+++ etc/NEWS    2011-03-22 01:48:22 +0000
@@ -649,6 +649,9 @@
 **** Packages using Log View mode can enable this functionality by
 binding `log-view-expanded-log-entry-function' to a suitable function.
 
+*** New command `vc-ediff' allows visual comparison of two revisions
+of a file similar to `vc-diff', but using ediff backend.
+
 ** Miscellaneous
 
 ---

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog      2011-03-22 00:30:23 +0000
+++ lisp/ChangeLog      2011-03-22 04:36:30 +0000
@@ -1,3 +1,15 @@
+2011-03-22  Christoph Scholtes  <address@hidden>
+
+       * vc/ediff-vers.el (ediff-revision-internal): New
+       function. Factored out interface to ediff-revision without user
+       interaction.
+
+       * vc/ediff.el: Require ediff-vers (for ediff-revision-internal).
+       (ediff-revision): Use new function ediff-revision-internal.
+
+       * vc/vc.el (vc-ediff): New function. Provides functionality
+       similar to vc-diff using ediff backend.
+
 2011-03-22  Chong Yidong  <address@hidden>
 
        * custom.el (custom--inhibit-theme-enable): Make it affect only

=== modified file 'lisp/vc/ediff-vers.el'
--- lisp/vc/ediff-vers.el       2011-01-25 04:08:28 +0000
+++ lisp/vc/ediff-vers.el       2011-03-22 01:34:43 +0000
@@ -59,6 +59,14 @@
       'vc-working-revision
     'vc-workfile-version))
 
+
+(defun ediff-revision-internal (rev1 rev2 &optional startup-hook)
+  ;; Call backend-specific ediff function. Uses `vc.el' or `rcs.el'
+  ;; depending on `ediff-version-control-package'."
+  (funcall
+   (intern (format "ediff-%S-internal" ediff-version-control-package))
+   rev1 rev2 startup-hook))
+
 ;; VC.el support
 
 (eval-when-compile

=== modified file 'lisp/vc/ediff.el'
--- lisp/vc/ediff.el    2011-01-25 04:08:28 +0000
+++ lisp/vc/ediff.el    2011-03-22 03:08:42 +0000
@@ -120,7 +120,8 @@
 (eval-when-compile
   (require 'dired)
   (require 'ediff-util)
-  (require 'ediff-ptch))
+  (require 'ediff-ptch)
+  (require 'ediff-vers))
 ;; end pacifier
 
 (require 'ediff-init)
@@ -1435,10 +1436,7 @@
           (format "Revision 2 to compare (default %s's current state): "
                   (file-name-nondirectory file))))
     (ediff-load-version-control)
-    (funcall
-     (intern (format "ediff-%S-internal" ediff-version-control-package))
-     rev1 rev2 startup-hooks)
-    ))
+    (ediff-revision-internal rev1 rev2 startup-hooks)))
 
 
 ;;;###autoload

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el       2011-03-07 08:56:30 +0000
+++ lisp/vc/vc.el       2011-03-22 01:45:10 +0000
@@ -1681,6 +1681,35 @@
                      (called-interactively-p 'interactive))))
 
 ;;;###autoload
+(defun vc-ediff (historic &optional not-urgent)
+  "Display diffs between revisions of a file using ediff.
+Normally this compares the currently selected file with its
+working revision. With the prefix argument HISTORIC, it reads two revision
+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))
+  (when buffer-file-name (vc-buffer-sync not-urgent))
+  (let* ((vc-fileset (vc-deduce-fileset not-urgent))
+         (files (cadr vc-fileset))
+         (first (car files)))
+    (cond
+     ;; FIXME: Only supports one selected file (for now?).
+     ;; Alternatively, we could spin off a separate ediff session
+     ;; for each of the selected files.
+     ((= (length files) 1)
+      (if historic
+          ;; Let user select revisions to compare.
+          (ediff-revision first)
+        (find-file first)
+        ;; With empty arguments, function compares latest version of
+        ;; current buffer's file with current buffer.
+        (ediff-revision-internal "" "")))
+     (t
+      (error "Function does not support more than one file")))))
+
+;;;###autoload
 (defun vc-root-diff (historic &optional not-urgent)
   "Display diffs between VC-controlled whole tree revisions.
 Normally, this compares the tree corresponding to the current