From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Bob Rogers Newsgroups: gmane.emacs.bugs Subject: bug#7350: 24.0.50; make vc-deduce-backend smarter Date: Tue, 16 Nov 2010 23:43:48 -0500 Message-ID: <19683.23940.23038.507387@rgr.rgrjr.com> References: <19669.54304.980911.220632@rgr.rgrjr.com> <19671.7412.608640.593137@rgr.rgrjr.com> <19672.26151.150217.308712@rgr.rgrjr.com> <19680.28414.314610.707478@rgr.rgrjr.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1289969066 7455 80.91.229.12 (17 Nov 2010 04:44:26 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 17 Nov 2010 04:44:26 +0000 (UTC) Cc: 7350@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Nov 17 05:44:21 2010 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1PIZsa-0007YK-SI for geb-bug-gnu-emacs@m.gmane.org; Wed, 17 Nov 2010 05:44:21 +0100 Original-Received: from localhost ([127.0.0.1]:45367 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIZsa-0003I7-9B for geb-bug-gnu-emacs@m.gmane.org; Tue, 16 Nov 2010 23:44:20 -0500 Original-Received: from [140.186.70.92] (port=41534 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIZsV-0003I2-Jj for bug-gnu-emacs@gnu.org; Tue, 16 Nov 2010 23:44:16 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PIZsU-0008DY-2t for bug-gnu-emacs@gnu.org; Tue, 16 Nov 2010 23:44:15 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:40648) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PIZsT-0008DS-Uo for bug-gnu-emacs@gnu.org; Tue, 16 Nov 2010 23:44:14 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1PIZnS-0002UX-Bi; Tue, 16 Nov 2010 23:39:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Bob Rogers Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 17 Nov 2010 04:39:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 7350 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 7350-submit@debbugs.gnu.org id=B7350.12899687339571 (code B ref 7350); Wed, 17 Nov 2010 04:39:02 +0000 Original-Received: (at 7350) by debbugs.gnu.org; 17 Nov 2010 04:38:53 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PIZnH-0002UK-Mv for submit@debbugs.gnu.org; Tue, 16 Nov 2010 23:38:52 -0500 Original-Received: from rgrjr.com ([216.146.47.5]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PIZnF-0002UF-By for 7350@debbugs.gnu.org; Tue, 16 Nov 2010 23:38:50 -0500 Original-Received: from rgrjr.dyndns.org (c-66-30-196-77.hsd1.ma.comcast.net [66.30.196.77]) by rgrjr.com (Postfix on CentOS) with ESMTP id BFE5F16018D for <7350@debbugs.gnu.org>; Wed, 17 Nov 2010 04:43:49 +0000 (UTC) Original-Received: (qmail 12650 invoked by uid 89); 17 Nov 2010 04:43:49 -0000 Original-Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1) by home with SMTP; 17 Nov 2010 04:43:49 -0000 Original-Received: by rgr.rgrjr.com (Postfix, from userid 500) id 7CD0AA4E6E; Tue, 16 Nov 2010 23:43:48 -0500 (EST) In-Reply-To: X-Mailer: VM 7.19 under Emacs 24.0.50.1 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Tue, 16 Nov 2010 23:39:02 -0500 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:41681 Archived-At: From: Stefan Monnier 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