From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: New function: vc-ediff Date: Sun, 27 Mar 2011 16:58:23 -0400 Message-ID: References: <4D785193.3030201@gmail.com> <4D79A736.6080707@gmail.com> <831v2cu40y.fsf@gnu.org> <838vwkrroz.fsf@gnu.org> <86r5a3j1a1.fsf@gmail.com> <86ipvbdaoq.fsf@gmail.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1301259521 6256 80.91.229.12 (27 Mar 2011 20:58:41 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 27 Mar 2011 20:58:41 +0000 (UTC) Cc: Eli Zaretskii , emacs-devel@gnu.org To: Christoph Scholtes Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Mar 27 22:58:34 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Q3x2f-0008Py-GM for ged-emacs-devel@m.gmane.org; Sun, 27 Mar 2011 22:58:33 +0200 Original-Received: from localhost ([127.0.0.1]:49264 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q3x2e-00078e-Iq for ged-emacs-devel@m.gmane.org; Sun, 27 Mar 2011 16:58:32 -0400 Original-Received: from [140.186.70.92] (port=50922 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q3x2Z-00078R-2G for emacs-devel@gnu.org; Sun, 27 Mar 2011 16:58:28 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q3x2Y-0000Ew-4Q for emacs-devel@gnu.org; Sun, 27 Mar 2011 16:58:27 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:7002 helo=ironport2-out.pppoe.ca) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q3x2W-0000EX-SC; Sun, 27 Mar 2011 16:58:24 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEAD6kj01MCqRC/2dsb2JhbAClUHiIa7c2hWkElgU X-IronPort-AV: E=Sophos;i="4.63,251,1299474000"; d="scan'208";a="98274732" Original-Received: from 76-10-164-66.dsl.teksavvy.com (HELO pastel.home) ([76.10.164.66]) by ironport2-out.pppoe.ca with ESMTP/TLS/ADH-AES256-SHA; 27 Mar 2011 16:58:23 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 3CA6458EBD; Sun, 27 Mar 2011 16:58:23 -0400 (EDT) In-Reply-To: <86ipvbdaoq.fsf@gmail.com> (Christoph Scholtes's message of "Mon, 21 Mar 2011 22:46:29 -0600") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:137765 Archived-At: > 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 > + > + * 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