New function: vc-ediff

New function: vc-ediff

[Christoph Scholtes]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

I started using ediff recently and was missing an easy way to compare 
the current buffer (or a file selected in vc-dir) with its latest 
revision in the repo. Basically, what vc-diff does, only using ediff 
instead.

Attached is a first cut at implementing this. Note, that this patch is 
just for review of the basic approach. I will add Changelog entries and 
(if applicable) manual documentation etc. later, if the approach has 
been reviewed.

Any comments are appreciated.

Thanks,
Christoph

[-- Attachment #2: vc-ediff_v1.patch --]
[-- Type: text/plain, Size: 3248 bytes --]

=== 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-10 04:03:04 +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-10 04:03:14 +0000
@@ -1408,8 +1408,7 @@
 (defun ediff-revision (&optional file startup-hooks)
   "Run Ediff by comparing versions of a file.
 The file is an optional FILE argument or the file entered at the prompt.
-Default: the file visited by the current buffer.
-Uses `vc.el' or `rcs.el' depending on `ediff-version-control-package'."
+Default: the file visited by the current buffer."
   ;; if buffer is non-nil, use that buffer instead of the current buffer
   (interactive "P")
   (if (not (stringp file))
@@ -1435,11 +1434,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
 (defalias 'erevision 'ediff-revision)

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2011-03-07 08:56:30 +0000
+++ lisp/vc/vc.el	2011-03-10 04:04:32 +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 "`vc-ediff' does not support selecting 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

Re: New function: vc-ediff

[Stefan Monnier]

> Attached is a first cut at implementing this. Note, that this patch is just

I think vc-ediff should not require any changes to ediff.
You can check pcvs.el for an example of code that provides similar
functionality without any ediff changes.


        Stefan

Re: New function: vc-ediff

[Michael Welsh Duggan]

Christoph Scholtes <cschol2112@googlemail.com> writes:

> I started using ediff recently and was missing an easy way to compare
> the current buffer (or a file selected in vc-dir) with its latest
> revision in the repo. Basically, what vc-diff does, only using ediff
> instead.

Is this different from `ediff-revision'?

-- 
Michael Welsh Duggan
(md5i@md5i.com)

Re: New function: vc-ediff

[Christoph Scholtes]

Hi Stefan,

On 3/10/2011 9:00 AM, Stefan Monnier wrote:

> I think vc-ediff should not require any changes to ediff.
> You can check pcvs.el for an example of code that provides similar
> functionality without any ediff changes.

I saw that, but I honestly didn't like this implementation. I felt using 
`ediff-buffers' I would need to duplicate code that had already been 
written, namely in `ediff-revision'.

I figured, what I needed for 'vc-diff' is basically `ediff-revision', 
but without the interactive prompts for revisions. Unfortunately, the 
function does not work that way. So I factored out the interactive piece 
from the piece I would need for `vc-ediff'. The refactoring in 
`ediff.el' is minimal just exposes an internal, non-interactive function 
for other functions besides `ediff-revision' to use.

Ultimately, `ediff-(vc|rcs)-internal', which is called by 
`ediff-revision', uses `ediff-buffers', like the implementation in 
`pcvs.el'.

Is there any specific reason why the refactoring in `ediff.el' should 
not be done?

Christoph

Re: New function: vc-ediff

[Christoph Scholtes]

On 3/10/2011 9:12 PM, Michael Welsh Duggan wrote:

> Is this different from `ediff-revision'?

No, except that I want it to work non-interactive, like `vc-diff' from 
`vc-dired'. In fact, my proposal uses a non-interactive version of 
`ediff-revision' to create `vc-diff'.

Christoph

Re: New function: vc-ediff

[Uday S Reddy]

On 3/11/2011 4:12 AM, Michael Welsh Duggan wrote:

>
> Is this different from `ediff-revision'?

`ediff-revision' asks just a few too many questions.  Most of the time, 
you just want to compare the current buffer with the latest revision. 
So, I find myself having to hit a lot of RETs when I use `ediff-revision'.

ediff's defaults in terms of ediff-last-use-dir are also not right in 
the context of vc.  I was burned by them on a few occasions.  You think 
you are comparing the current buffer with its revision, but ediff would 
have gone and picked up a file with the same name in the last-use-dir 
and started comparing it with its revision.  If you maintain multiple 
development lines, this might happen more often than one might expect, 
and the results can be quite confusing and even dangerous.

The user interface turns out to be just a bit too important in this case.

Cheers,
Uday

Re: New function: vc-ediff

[martin rudalics]

>> Is this different from `ediff-revision'?
> 
> `ediff-revision' asks just a few too many questions.  Most of the time, 
> you just want to compare the current buffer with the latest revision. 
> So, I find myself having to hit a lot of RETs when I use `ediff-revision'.

I agree with Uday - it's annoying.

martin

Re: New function: vc-ediff

[Stefan Monnier]

>> I think vc-ediff should not require any changes to ediff.
>> You can check pcvs.el for an example of code that provides similar
>> functionality without any ediff changes.
> I saw that, but I honestly didn't like this implementation. I felt using
> ediff-buffers' I would need to duplicate code that had already been written,
> namely in `ediff-revision'.

I guess it's OK if Michael likes it.  I happen to think that vc-ediff
should *replace* ediff-revision ;-)


        Stefan

Re: New function: vc-ediff

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Fri, 11 Mar 2011 15:15:43 -0500
> Cc: emacs-devel@gnu.org
> 
> I happen to think that vc-ediff should *replace* ediff-revision ;-)

It could, if it is sensitive enough to the context.  When the command
is invoked in the context of some VC-related feature, like from the
*vc-dir* buffer, it should intuit most of the parameters and not ask
too many questions, because the user would be annoyed.  But when the
command is invoked out of VC context, like from some unrelated buffer,
it might be a good idea to ask, because the user could have something
very different in mind than to diff the current buffer's file.

IIUC, vc-ediff, as written, does not allow me to specify the file for
which to show the diffs (unless I invoke it with a prefix argument).
It assumes that I want to diff the file visited by the current
buffer.  I'm not sure this is the best default in this case.

Re: New function: vc-ediff

[Christoph Scholtes]

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> But when the command is invoked out of VC context, like from some
> unrelated buffer, it might be a good idea to ask, because the user
> could have something very different in mind than to diff the current
> buffer's file.

What do you mean by `out of VC context'?

> IIUC, vc-ediff, as written, does not allow me to specify the file for
> which to show the diffs (unless I invoke it with a prefix argument).
> It assumes that I want to diff the file visited by the current
> buffer.  I'm not sure this is the best default in this case.

My initial intention was to provide an `ediff' equivalent to `vc-diff',
which works on what has been deduced by `vc-deduce-fileset', i.e. the
current buffer or the selection in `vc-dir'.

Christoph

Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I guess it's OK if Michael likes it.

Michael Kifer?

> I happen to think that vc-ediff should *replace* ediff-revision ;-)

Interesting. Is it to separate the version control aspect from ediff?

Christoph

Re: New function: vc-ediff

[Eli Zaretskii]

> From: Christoph Scholtes <cschol2112@googlemail.com>
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>,  emacs-devel@gnu.org
> Date: Sat, 12 Mar 2011 08:05:56 -0700
> 
> > But when the command is invoked out of VC context, like from some
> > unrelated buffer, it might be a good idea to ask, because the user
> > could have something very different in mind than to diff the current
> > buffer's file.
> 
> What do you mean by `out of VC context'?

In a buffer that doesn't visit a versioned file for which Emacs can
find the corresponding repository.

> > IIUC, vc-ediff, as written, does not allow me to specify the file for
> > which to show the diffs (unless I invoke it with a prefix argument).
> > It assumes that I want to diff the file visited by the current
> > buffer.  I'm not sure this is the best default in this case.
> 
> My initial intention was to provide an `ediff' equivalent to `vc-diff',
> which works on what has been deduced by `vc-deduce-fileset', i.e. the
> current buffer or the selection in `vc-dir'.

That's fine.  I was replying to Stefan's wish that vc-ediff _replaces_
ediff-version.  For that, IMO its UI should support invocation outside
of VC context.

Re: New function: vc-ediff

[Stefan Monnier]

> IIUC, vc-ediff, as written, does not allow me to specify the file for
> which to show the diffs (unless I invoke it with a prefix argument).

You should be able to see the (e)diff for any version controlled file
(for backends supported by VC), using vc-dir.  If not, that's
a serious shortcoming.

But my point is not about UI, it's about implementation: I think ediff
should not call VC functions.


        Stefan

Re: New function: vc-ediff

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: cschol2112@googlemail.com,  emacs-devel@gnu.org
> Date: Sat, 12 Mar 2011 15:44:48 -0500
> 
> You should be able to see the (e)diff for any version controlled file
> (for backends supported by VC), using vc-dir.

What, you mean I cannot see the diffs against a previous version of a
file unless I run vc-dir on its directory?

Re: New function: vc-ediff

[Stefan Monnier]

>> I happen to think that vc-ediff should *replace* ediff-revision ;-)
> Interesting. Is it to separate the version control aspect from ediff?

Yes.


        Stefan

Re: New function: vc-ediff

[Stefan Monnier]

>> You should be able to see the (e)diff for any version controlled file
>> (for backends supported by VC), using vc-dir.
> What, you mean I cannot see the diffs against a previous version of a
> file unless I run vc-dir on its directory?

I don't even know if it can.  It's not relevant to the question at hand:
there is no diff-revisions, there's only vc-diff; so if vc-diff is
sufficient then so is vc-ediff.


        Stefan

Re: New function: vc-ediff

[Uday S Reddy]

On 3/12/2011 8:48 PM, Eli Zaretskii wrote:

>
> What, you mean I cannot see the diffs against a previous version of a
> file unless I run vc-dir on its directory?

The proposed vc-ediff works when visiting any version-controlled file. 
No need for vc-dir (which I have never used, really).

Cheers,
Uday

Re: New function: vc-ediff

[Eli Zaretskii]

> From: Uday S Reddy <u.s.reddy@cs.bham.ac.uk>
> Date: Sun, 13 Mar 2011 16:16:54 +0000
> 
> On 3/12/2011 8:48 PM, Eli Zaretskii wrote:
> 
> >
> > What, you mean I cannot see the diffs against a previous version of a
> > file unless I run vc-dir on its directory?
> 
> The proposed vc-ediff works when visiting any version-controlled file. 

What if I don't visit a version-controlled file, and still want to
compare some file against some of its versions?

Re: New function: vc-ediff

[Stefan Monnier]

>> > What, you mean I cannot see the diffs against a previous version of a
>> > file unless I run vc-dir on its directory?
>> The proposed vc-ediff works when visiting any version-controlled file. 
> What if I don't visit a version-controlled file, and still want to
> compare some file against some of its versions?

As mentioned, this is just as true of vc-diff as of vc-ediff, so file
a bug-report against vc-diff if you think that's a problem.
Personally I think that vc-dir is a perfectly acceptable solution.


        Stefan

Re: New function: vc-ediff

[Christoph Scholtes]

On 3/13/2011 12:01 PM, Eli Zaretskii wrote:

>> The proposed vc-ediff works when visiting any version-controlled file.
>
> What if I don't visit a version-controlled file, and still want to
> compare some file against some of its versions?

I would visit the file I want to compare first and then invoke vc-ediff. 
To me, that's not much different than calling a version of vc-ediff, 
which asks me first which file to compare, like ediff-revision does.

However, if this is really a desired feature, we could enhance 
vc-deduce-fileset to not just fail with `no fileset found' on being 
invoked on a buffer that is not version controlled, but ask for a file. 
This would then also work for vc-diff and keep everything consistent.

Christoph

Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> You should be able to see the (e)diff for any version controlled file
>>> (for backends supported by VC), using vc-dir.
>> What, you mean I cannot see the diffs against a previous version of a
>> file unless I run vc-dir on its directory?
>
> I don't even know if it can.  It's not relevant to the question at hand:
> there is no diff-revisions, there's only vc-diff; so if vc-diff is
> sufficient then so is vc-ediff.

So, what's the verdict on this issue? Is it acceptable the way it was
originally proposed or should I try to implement vc-ediff as a
replacement for ediff-revision?

Christoph

Re: New function: vc-ediff

[Stefan Monnier]

>>>> You should be able to see the (e)diff for any version controlled file
>>>> (for backends supported by VC), using vc-dir.
>>> What, you mean I cannot see the diffs against a previous version of a
>>> file unless I run vc-dir on its directory?
>> I don't even know if it can.  It's not relevant to the question at hand:
>> there is no diff-revisions, there's only vc-diff; so if vc-diff is
>> sufficient then so is vc-ediff.
> So, what's the verdict on this issue? Is it acceptable the way it was
> originally proposed or should I try to implement vc-ediff as a
> replacement for ediff-revision?

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.
Can you submit a patch according to these guidelines?


        Stefan

Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <monnier@iro.umontreal.ca> 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  <cschol2112@googlemail.com>
+
+	* maintaining.texi (Old Revisions): Add paragraph on new function
+	vc-ediff.
+
 2011-03-12  Eli Zaretskii  <eliz@gnu.org>
 
 	* 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.
 
+@findex 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  <cschol2112@googlemail.com>
+
+	* NEWS: Document new function `vc-ediff'.
+
 2011-03-16  Juanma Barranquero  <lekktu@gmail.com>
 
 	* 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  <cschol2112@googlemail.com>
+
+	* 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  <cyd@stupidchicken.com>
 
 	* 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

Re: New function: vc-ediff

[Christoph Scholtes]

Christoph Scholtes <cschol2112@googlemail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Can you submit a patch according to these guidelines?
>
> I cleaned up the patch and also updated Changelogs, NEWS and manual.

Did you have any comments on this patch?

Thanks,
Christoph

Re: New function: vc-ediff

[Stefan Monnier]

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



> +@findex 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
                                                            ^^^
                                                            in?

> +2011-03-22  Christoph Scholtes  <cschol2112@googlemail.com>
> +
> +	* 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.

No need to give any detail about a new function's purpose, just "New
function" is enough: the source code should explain the purpose (either
self-evidently or via comments&docstrings).
Also, remove the empty lines to make it clear that those three file-changes
go together.

> +(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))

I think this is not needed.

> === 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)))
 
If we don't need ediff-revision-internal then I think the above changes
can also be dropped.

> +(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)

Problem with this code is that you depend on ediff-revision to get
the revision arguments.  E.g. this doesn't provide completion.
IOW we should use more of vc-diff's code here.

> +        (find-file first)
> +        ;; With empty arguments, function compares latest version of
> +        ;; current buffer's file with current buffer.
> +        (ediff-revision-internal "" "")))

Call ediff-vc-internal here.


        Stefan

Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +@findex 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
>                                                             ^^^
>                                                             in?

Yes. Fixed.

> No need to give any detail about a new function's purpose, just "New
> function" is enough: the source code should explain the purpose (either
> self-evidently or via comments&docstrings).
> Also, remove the empty lines to make it clear that those three file-changes
> go together.

OK. Fixed also.

> Problem with this code is that you depend on ediff-revision to get
> the revision arguments.  E.g. this doesn't provide completion.
> IOW we should use more of vc-diff's code here.

It was my intention to reuse the code that ediff-revision already
provides to get the revision arguments. I am not sure I understand what
you mean by "does not provide completion".

Do you mean vc-diff's code to get the revision arguments?

Christoph

Re: New function: vc-ediff

[Stefan Monnier]

> It was my intention to reuse the code that ediff-revision already
> provides to get the revision arguments. I am not sure I understand what
> you mean by "does not provide completion".

Try in a bzr checkout C-u C-x v = a:../ TAB TAB

For RCS/CVS revision numbers, completion is not very useful, but for
Bazaar, Monotone, and many others, it's an important functionality.


        Stefan

Re: New function: vc-ediff

[Christoph Scholtes]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It was my intention to reuse the code that ediff-revision already
>> provides to get the revision arguments. I am not sure I understand what
>> you mean by "does not provide completion".
>
> Try in a bzr checkout C-u C-x v = a:../ TAB TAB
>
> For RCS/CVS revision numbers, completion is not very useful, but for
> Bazaar, Monotone, and many others, it's an important functionality.

I see. I never knew this was possible. I will investigate another
solution then that uses `vc-diff'. 

Thanks for the feedback.

Christoph

Re: New function: vc-ediff

[Christoph Scholtes]

[-- Attachment #1: Type: text/plain, Size: 147 bytes --]

Hi Stefan,

Here is a new version of the `vc-ediff' patch. No changes on the `ediff' 
side, all new functions and refactoring in vc.el.

Christoph

[-- Attachment #2: vc-diff_v3.patch --]
[-- Type: text/plain, Size: 8503 bytes --]

=== modified file 'doc/emacs/ChangeLog'
--- doc/emacs/ChangeLog	2011-04-06 12:18:10 +0000
+++ doc/emacs/ChangeLog	2011-04-08 01:10:01 +0000
@@ -1,3 +1,8 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* maintaining.texi (Old Revisions): Add paragraph on new function
+	vc-ediff.
+
 2011-03-30  Eli Zaretskii  <eliz@gnu.org>
 
 	* display.texi (Auto Scrolling): Document the limit of 100 lines

=== modified file 'doc/emacs/maintaining.texi'
--- doc/emacs/maintaining.texi	2011-03-01 04:14:00 +0000
+++ doc/emacs/maintaining.texi	2011-04-08 01:09:15 +0000
@@ -744,6 +744,13 @@
 buffer, these commands generate a diff of all registered files in the
 current directory and its subdirectories.
 
+@findex vc-ediff
+The function @code{vc-ediff} works like @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.
+
 @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-04-06 19:38:46 +0000
+++ etc/ChangeLog	2011-04-08 01:06:15 +0000
@@ -1,3 +1,7 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* NEWS: Document new function `vc-ediff'.
+
 2011-04-06  Juanma Barranquero  <lekktu@gmail.com>
 
 	* NEWS: New variable `revert-buffer-in-progress-p'.

=== modified file 'etc/NEWS'
--- etc/NEWS	2011-04-06 20:10:51 +0000
+++ etc/NEWS	2011-04-08 01:05:55 +0000
@@ -670,6 +670,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-04-07 03:27:15 +0000
+++ lisp/ChangeLog	2011-04-08 00:55:54 +0000
@@ -1,3 +1,9 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* vc/vc.el (vc-diff-build-argument-list-internal)
+	(vc-version-ediff, vc-ediff): New functions.
+	(vc-version-diff): Use vc-diff-build-argument-list-internal.
+
 2011-04-07  Aaron S. Hawley  <aaron.s.hawley@gmail.com>
 
 	* play/morse.el (denato-region): Handle varying case.  (Bug#8386)

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2011-03-07 08:56:30 +0000
+++ lisp/vc/vc.el	2011-04-08 00:57:37 +0000
@@ -653,6 +653,7 @@
 
 (require 'vc-hooks)
 (require 'vc-dispatcher)
+(require 'ediff)
 
 (eval-when-compile
   (require 'cl)
@@ -1617,45 +1618,48 @@
                          nil nil initial-input nil default)
       (read-string prompt initial-input nil default))))
 
+(defun vc-diff-build-argument-list-internal ()
+  "Build argument list for calling internal diff functions."
+  (let* ((vc-fileset (vc-deduce-fileset t)) ;FIXME: why t?  --Stef
+         (files (cadr vc-fileset))
+         (backend (car vc-fileset))
+         (first (car files))
+         (rev1-default nil)
+         (rev2-default nil))
+    (cond
+     ;; someday we may be able to do revision completion on non-singleton
+     ;; 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 and previous revisions as defaults
+     (t
+      (setq rev1-default (vc-call-backend backend 'previous-revision first
+                                          (vc-working-revision first)))
+      (when (string= rev1-default "") (setq rev1-default nil))
+      (setq rev2-default (vc-working-revision first))))
+    ;; construct argument list
+    (let* ((rev1-prompt (if rev1-default
+                            (concat "Older revision (default "
+                                    rev1-default "): ")
+                          "Older revision: "))
+           (rev2-prompt (concat "Newer revision (default "
+                                (or rev2-default "current source") "): "))
+           (rev1 (vc-read-revision rev1-prompt files backend rev1-default))
+           (rev2 (vc-read-revision rev2-prompt files backend rev2-default)))
+      (when (string= rev1 "") (setq rev1 nil))
+      (when (string= rev2 "") (setq rev2 nil))
+      (list files rev1 rev2))))
+
 ;;;###autoload
 (defun vc-version-diff (files rev1 rev2)
   "Report diffs between revisions of the fileset in the repository history."
-  (interactive
-   (let* ((vc-fileset (vc-deduce-fileset t)) ;FIXME: why t?  --Stef
-	  (files (cadr vc-fileset))
-          (backend (car vc-fileset))
-	  (first (car files))
-	  (rev1-default nil)
-	  (rev2-default nil))
-     (cond
-      ;; someday we may be able to do revision completion on non-singleton
-      ;; 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 and previous revisions as defaults
-      (t
-       (setq rev1-default (vc-call-backend backend 'previous-revision first
-                                           (vc-working-revision first)))
-       (when (string= rev1-default "") (setq rev1-default nil))
-       (setq rev2-default (vc-working-revision first))))
-     ;; construct argument list
-     (let* ((rev1-prompt (if rev1-default
-			     (concat "Older revision (default "
-				     rev1-default "): ")
-			   "Older revision: "))
-	    (rev2-prompt (concat "Newer revision (default "
-				 (or rev2-default "current source") "): "))
-	    (rev1 (vc-read-revision rev1-prompt files backend rev1-default))
-	    (rev2 (vc-read-revision rev2-prompt files backend rev2-default)))
-       (when (string= rev1 "") (setq rev1 nil))
-       (when (string= rev2 "") (setq rev2 nil))
-       (list files rev1 rev2))))
+  (interactive (vc-diff-build-argument-list-internal))
   ;; All that was just so we could do argument completion!
   (when (and (not rev1) rev2)
     (error "Not a valid revision range"))
@@ -1681,6 +1685,48 @@
 		      (called-interactively-p 'interactive))))
 
 ;;;###autoload
+(defun vc-version-ediff (files rev1 rev2)
+  "Show differences between revisions of the fileset in the
+repository history using ediff."
+  (interactive (vc-diff-build-argument-list-internal))
+  ;; All that was just so we could do argument completion!
+  (when (and (not rev1) rev2)
+    (error "Not a valid revision range"))
+
+  (message "%s" (format "Finding changes in %s..." (vc-delistify files)))
+
+  ;; Functions ediff-(vc|rcs)-internal use "" instead of nil.
+  (when (null rev1) (setq rev1 ""))
+  (when (null rev2) (setq rev2 ""))
+
+  (cond
+   ;; FIXME We only support running ediff on one file for now.
+   ;; We could spin off an ediff session per file in the file set.
+   ((= (length files) 1)
+    (ediff-load-version-control)
+    (find-file (car files))
+    (funcall
+     (intern (format "ediff-%S-internal" ediff-version-control-package))
+     rev1 rev2 nil))
+   (t
+    (error "More than one file is not supported"))))
+
+;;;###autoload
+(defun vc-ediff (historic &optional not-urgent)
+  "Display diffs between file revisions using ediff.
+Normally this compares the currently selected fileset with their
+working revisions.  With a 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))
+  (if historic
+      (call-interactively 'vc-version-ediff)
+    (when buffer-file-name (vc-buffer-sync not-urgent))
+    (vc-version-ediff (cadr (vc-deduce-fileset t)) nil nil)))
+
+;;;###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

Re: New function: vc-ediff

[Christoph Scholtes]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

On 4/7/2011 7:16 PM, Christoph Scholtes wrote:

> Here is a new version of the `vc-ediff' patch. No changes on the `ediff'
> side, all new functions and refactoring in vc.el.

Ping.

Christoph


[-- Attachment #2: vc-diff_v3.patch --]
[-- Type: text/plain, Size: 8503 bytes --]

=== modified file 'doc/emacs/ChangeLog'
--- doc/emacs/ChangeLog	2011-04-06 12:18:10 +0000
+++ doc/emacs/ChangeLog	2011-04-08 01:10:01 +0000
@@ -1,3 +1,8 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* maintaining.texi (Old Revisions): Add paragraph on new function
+	vc-ediff.
+
 2011-03-30  Eli Zaretskii  <eliz@gnu.org>
 
 	* display.texi (Auto Scrolling): Document the limit of 100 lines

=== modified file 'doc/emacs/maintaining.texi'
--- doc/emacs/maintaining.texi	2011-03-01 04:14:00 +0000
+++ doc/emacs/maintaining.texi	2011-04-08 01:09:15 +0000
@@ -744,6 +744,13 @@
 buffer, these commands generate a diff of all registered files in the
 current directory and its subdirectories.
 
+@findex vc-ediff
+The function @code{vc-ediff} works like @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.
+
 @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-04-06 19:38:46 +0000
+++ etc/ChangeLog	2011-04-08 01:06:15 +0000
@@ -1,3 +1,7 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* NEWS: Document new function `vc-ediff'.
+
 2011-04-06  Juanma Barranquero  <lekktu@gmail.com>
 
 	* NEWS: New variable `revert-buffer-in-progress-p'.

=== modified file 'etc/NEWS'
--- etc/NEWS	2011-04-06 20:10:51 +0000
+++ etc/NEWS	2011-04-08 01:05:55 +0000
@@ -670,6 +670,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-04-07 03:27:15 +0000
+++ lisp/ChangeLog	2011-04-08 00:55:54 +0000
@@ -1,3 +1,9 @@
+2011-04-08  Christoph Scholtes  <cschol2112@googlemail.com>
+
+	* vc/vc.el (vc-diff-build-argument-list-internal)
+	(vc-version-ediff, vc-ediff): New functions.
+	(vc-version-diff): Use vc-diff-build-argument-list-internal.
+
 2011-04-07  Aaron S. Hawley  <aaron.s.hawley@gmail.com>
 
 	* play/morse.el (denato-region): Handle varying case.  (Bug#8386)

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2011-03-07 08:56:30 +0000
+++ lisp/vc/vc.el	2011-04-08 00:57:37 +0000
@@ -653,6 +653,7 @@
 
 (require 'vc-hooks)
 (require 'vc-dispatcher)
+(require 'ediff)
 
 (eval-when-compile
   (require 'cl)
@@ -1617,45 +1618,48 @@
                          nil nil initial-input nil default)
       (read-string prompt initial-input nil default))))
 
+(defun vc-diff-build-argument-list-internal ()
+  "Build argument list for calling internal diff functions."
+  (let* ((vc-fileset (vc-deduce-fileset t)) ;FIXME: why t?  --Stef
+         (files (cadr vc-fileset))
+         (backend (car vc-fileset))
+         (first (car files))
+         (rev1-default nil)
+         (rev2-default nil))
+    (cond
+     ;; someday we may be able to do revision completion on non-singleton
+     ;; 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 and previous revisions as defaults
+     (t
+      (setq rev1-default (vc-call-backend backend 'previous-revision first
+                                          (vc-working-revision first)))
+      (when (string= rev1-default "") (setq rev1-default nil))
+      (setq rev2-default (vc-working-revision first))))
+    ;; construct argument list
+    (let* ((rev1-prompt (if rev1-default
+                            (concat "Older revision (default "
+                                    rev1-default "): ")
+                          "Older revision: "))
+           (rev2-prompt (concat "Newer revision (default "
+                                (or rev2-default "current source") "): "))
+           (rev1 (vc-read-revision rev1-prompt files backend rev1-default))
+           (rev2 (vc-read-revision rev2-prompt files backend rev2-default)))
+      (when (string= rev1 "") (setq rev1 nil))
+      (when (string= rev2 "") (setq rev2 nil))
+      (list files rev1 rev2))))
+
 ;;;###autoload
 (defun vc-version-diff (files rev1 rev2)
   "Report diffs between revisions of the fileset in the repository history."
-  (interactive
-   (let* ((vc-fileset (vc-deduce-fileset t)) ;FIXME: why t?  --Stef
-	  (files (cadr vc-fileset))
-          (backend (car vc-fileset))
-	  (first (car files))
-	  (rev1-default nil)
-	  (rev2-default nil))
-     (cond
-      ;; someday we may be able to do revision completion on non-singleton
-      ;; 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 and previous revisions as defaults
-      (t
-       (setq rev1-default (vc-call-backend backend 'previous-revision first
-                                           (vc-working-revision first)))
-       (when (string= rev1-default "") (setq rev1-default nil))
-       (setq rev2-default (vc-working-revision first))))
-     ;; construct argument list
-     (let* ((rev1-prompt (if rev1-default
-			     (concat "Older revision (default "
-				     rev1-default "): ")
-			   "Older revision: "))
-	    (rev2-prompt (concat "Newer revision (default "
-				 (or rev2-default "current source") "): "))
-	    (rev1 (vc-read-revision rev1-prompt files backend rev1-default))
-	    (rev2 (vc-read-revision rev2-prompt files backend rev2-default)))
-       (when (string= rev1 "") (setq rev1 nil))
-       (when (string= rev2 "") (setq rev2 nil))
-       (list files rev1 rev2))))
+  (interactive (vc-diff-build-argument-list-internal))
   ;; All that was just so we could do argument completion!
   (when (and (not rev1) rev2)
     (error "Not a valid revision range"))
@@ -1681,6 +1685,48 @@
 		      (called-interactively-p 'interactive))))
 
 ;;;###autoload
+(defun vc-version-ediff (files rev1 rev2)
+  "Show differences between revisions of the fileset in the
+repository history using ediff."
+  (interactive (vc-diff-build-argument-list-internal))
+  ;; All that was just so we could do argument completion!
+  (when (and (not rev1) rev2)
+    (error "Not a valid revision range"))
+
+  (message "%s" (format "Finding changes in %s..." (vc-delistify files)))
+
+  ;; Functions ediff-(vc|rcs)-internal use "" instead of nil.
+  (when (null rev1) (setq rev1 ""))
+  (when (null rev2) (setq rev2 ""))
+
+  (cond
+   ;; FIXME We only support running ediff on one file for now.
+   ;; We could spin off an ediff session per file in the file set.
+   ((= (length files) 1)
+    (ediff-load-version-control)
+    (find-file (car files))
+    (funcall
+     (intern (format "ediff-%S-internal" ediff-version-control-package))
+     rev1 rev2 nil))
+   (t
+    (error "More than one file is not supported"))))
+
+;;;###autoload
+(defun vc-ediff (historic &optional not-urgent)
+  "Display diffs between file revisions using ediff.
+Normally this compares the currently selected fileset with their
+working revisions.  With a 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))
+  (if historic
+      (call-interactively 'vc-version-ediff)
+    (when buffer-file-name (vc-buffer-sync not-urgent))
+    (vc-version-ediff (cadr (vc-deduce-fileset t)) nil nil)))
+
+;;;###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

Re: New function: vc-ediff

[Stefan Monnier]

>> Here is a new version of the `vc-ediff' patch. No changes on the `ediff'
>> side, all new functions and refactoring in vc.el.
> Ping.

Thanks, installed, with the subsequent tweaks below.


        Stefan


=== modified file 'doc/emacs/ChangeLog'
--- doc/emacs/ChangeLog	2011-04-20 17:33:09 +0000
+++ doc/emacs/ChangeLog	2011-04-20 17:34:11 +0000
@@ -1,7 +1,6 @@
 2011-04-20  Christoph Scholtes  <cschol2112@googlemail.com>
 
-	* maintaining.texi (Old Revisions): Add paragraph on new function
-	vc-ediff.
+	* maintaining.texi (Old Revisions): Mention new function vc-ediff.
 
 2011-03-26  Chong Yidong  <cyd@stupidchicken.com>
 

=== modified file 'doc/emacs/maintaining.texi'
--- doc/emacs/maintaining.texi	2011-04-20 17:33:09 +0000
+++ doc/emacs/maintaining.texi	2011-04-20 17:34:38 +0000
@@ -746,10 +746,10 @@
 
 @findex vc-ediff
 The function @code{vc-ediff} works like @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.
+visually compare two revisions of a file in 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.
 
 @vindex vc-diff-switches
 @vindex vc-rcs-diff-switches

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2011-04-20 17:33:09 +0000
+++ lisp/ChangeLog	2011-04-20 17:39:15 +0000
@@ -1,7 +1,12 @@
+2011-04-20  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+	* vc/vc.el (vc-version-ediff): Call ediff-vc-internal directly, since
+	we're in VC after all.
+
 2011-04-20  Christoph Scholtes  <cschol2112@googlemail.com>
 
 	* vc/vc.el (vc-diff-build-argument-list-internal)
-	(vc-version-ediff, vc-ediff): New functions.
+	(vc-version-ediff, vc-ediff): New commands.
 	(vc-version-diff): Use vc-diff-build-argument-list-internal.
 
 2011-04-20  Stefan Monnier  <monnier@iro.umontreal.ca>

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2011-04-20 17:33:09 +0000
+++ lisp/vc/vc.el	2011-04-20 17:37:18 +0000
@@ -1704,10 +1704,8 @@
    ;; We could spin off an ediff session per file in the file set.
    ((= (length files) 1)
     (ediff-load-version-control)
-    (find-file (car files))
-    (funcall
-     (intern (format "ediff-%S-internal" ediff-version-control-package))
-     rev1 rev2 nil))
+    (find-file (car files))             ;FIXME: find-file from Elisp is bad.
+    (ediff-vc-internal rev1 rev2 nil))
    (t
     (error "More than one file is not supported"))))
 

Re: New function: vc-ediff

[Christoph Scholtes]

On 4/20/2011 11:39 AM, Stefan Monnier wrote:
>>> Here is a new version of the `vc-ediff' patch. No changes on the `ediff'
>>> side, all new functions and refactoring in vc.el.
>> Ping.
>
> Thanks, installed, with the subsequent tweaks below.

Cool, thank you!

Christoph