From: Bob Rogers <rogers@rgrjr.dyndns.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 7350@debbugs.gnu.org
Subject: bug#7350: 24.0.50; make vc-deduce-backend smarter
Date: Tue, 16 Nov 2010 23:43:48 -0500 [thread overview]
Message-ID: <19683.23940.23038.507387@rgr.rgrjr.com> (raw)
In-Reply-To: <jwvy68uinjf.fsf-monnier+emacs@gnu.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
next prev parent reply other threads:[~2010-11-17 4:43 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-06 22:18 bug#7350: 24.0.50; make vc-deduce-backend smarter Bob Rogers
2010-11-07 20:03 ` Stefan Monnier
2010-11-07 21:41 ` Bob Rogers
2010-11-08 17:47 ` Stefan Monnier
2010-11-08 21:05 ` Bob Rogers
2010-11-12 13:48 ` Stefan Monnier
2010-11-14 23:21 ` Bob Rogers
2010-11-15 16:05 ` Stefan Monnier
2010-11-17 4:43 ` Bob Rogers [this message]
2010-11-17 13:31 ` Stefan Monnier
2010-11-19 2:27 ` Bob Rogers
2010-11-20 19:54 ` Bob Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=19683.23940.23038.507387@rgr.rgrjr.com \
--to=rogers@rgrjr.dyndns.org \
--cc=7350@debbugs.gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.