From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dan Nicolaescu Newsgroups: gmane.emacs.devel Subject: Re: New VC diff for review Date: Sat, 06 Oct 2007 12:38:46 -0700 Message-ID: <200710061938.l96JcojN020804@oogie-boogie.ics.uci.edu> References: <20071006152353.GA15638@thyrsus.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1191699623 22883 80.91.229.12 (6 Oct 2007 19:40:23 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sat, 6 Oct 2007 19:40:23 +0000 (UTC) Cc: emacs-devel@gnu.org To: esr@thyrsus.com Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Oct 06 21:40:17 2007 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.50) id 1IeFVY-0003UZ-NW for ged-emacs-devel@m.gmane.org; Sat, 06 Oct 2007 21:40:17 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IeFVT-0007Bf-Jp for ged-emacs-devel@m.gmane.org; Sat, 06 Oct 2007 15:40:11 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IeFVO-00079C-Ht for emacs-devel@gnu.org; Sat, 06 Oct 2007 15:40:06 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IeFVN-00078C-Tx for emacs-devel@gnu.org; Sat, 06 Oct 2007 15:40:06 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IeFVN-000781-EU for emacs-devel@gnu.org; Sat, 06 Oct 2007 15:40:05 -0400 Original-Received: from oogie-boogie.ics.uci.edu ([128.195.1.41]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IeFVM-0007J1-T8 for emacs-devel@gnu.org; Sat, 06 Oct 2007 15:40:05 -0400 Original-Received: from mothra.ics.uci.edu (mothra.ics.uci.edu [128.195.6.93]) by oogie-boogie.ics.uci.edu (8.13.6/8.13.6) with ESMTP id l96JcojN020804; Sat, 6 Oct 2007 12:38:50 -0700 (PDT) In-Reply-To: <20071006152353.GA15638@thyrsus.com> (Eric S. Raymond's message of "Sat\, 6 Oct 2007 11\:23\:54 -0400") Original-Lines: 180 X-ICS-MailScanner: Found to be clean X-ICS-MailScanner-SpamCheck: not spam, SpamAssassin (score=-0.84, required 5, autolearn=disabled, ALL_TRUSTED -1.44, J_CHICKENPOX_22 0.60) X-ICS-MailScanner-From: dann@mothra.ics.uci.edu X-Detected-Kernel: Solaris 9 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:80342 Archived-At: Here are some comments on the patch: +;; $Id: vc.el,v 1.130 2007/10/06 14:54:22 esr Exp esr $ + Delete this @@ -1013,13 +1016,13 @@ ;; FIXME: file-relative-name can return a bogus result because ;; it doesn't look at the actual file-system to see if symlinks ;; come into play. - (let* ((files + (let* ((files ^^^^^^^^^^^^^^^^^^ Just whitespace differences, better avoid it. (mapcar (lambda (f) (file-relative-name (expand-file-name f))) (if (listp file-or-list) file-or-list (list file-or-list)))) - (full-command - (concat command " " (vc-delistify flags) " " (vc-delistify files)))) - (if vc-command-messages - (message "Running %s..." full-command)) + (full-command + (concat command " " + (vc-delistify (mapcar (lambda (s) (if (> (length s) 20) (concat (substring s 0 2) "") s)) flags)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Is this debugging code? If not please add a comment to say what it is doing because it is not obvious @@ -1065,11 +1071,12 @@ (shrink-window-if-larger-than-buffer) (error "Running %s...FAILED (%s)" full-command (if (integerp status) (format "status %d" status) status)))) + ;; We're done (if vc-command-messages - (message "Running %s...OK" full-command))) + (message "Running %s...OK = %d" full-command status))) (vc-exec-after - `(run-hook-with-args 'vc-post-command-functions - ',command ',file-or-list ',flags)) + `(run-hook-with-args 'vc-post-command-functions + ',command ',file-or-list ',flags)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Just whitespace differences, maybe better avoid it. - (if (not vc-log-operation) + (if (not vc-log-operation) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Just whitespace differences, maybe better avoid it. +;;;###autoload +(defun vc-history-diff (rev1 rev2) + "Report diffs between revisions of files in the repository history." + (interactive "sOlder revision: \nsNewer revision: ") + (let* ((files (vc-deduce-fileset t)) + (first (car files)) + (backend + (cond ((file-directory-p first) + (vc-responsible-backend first)) + (t + (vc-backend first))))) + (if (string= rev1 "") (setq rev1 nil)) + (if (string= rev2 "") (setq rev2 nil)) + (if (and (not rev1) rev2) + (error "Not a valid revision range.")) + (vc-diff-internal backend t files rev1 rev2 (interactive-p)))) One of these vc-diff* functions needs to do (vc-call revision-completion-table file) like vc-version-diff does in order to get completions for versions. (defun vc-version-other-window (rev) @@ -2277,7 +2244,6 @@ ((setq subdir (dired-get-subdir)) ;; if the backend supports it, get the state ;; of all files in this directory at once - (let ((backend (vc-responsible-backend subdir))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Isn't this line still needed, `backend' is still used below. ;; check `backend' can really handle `subdir'. (if (and (vc-call-backend backend 'responsible-p subdir) (vc-find-backend-function backend 'dir-state)) (vc-exec-after `(let ((inhibit-read-only t)) - (vc-call-backend ',(vc-backend file) 'log-view-mode) + (log-view-mode) ^^^^^^^^^^^^^^^^^^^^^ This is incorrect, the original is right. - (vc-checkout file nil "") + (vc-checkout file nil "") ^^^^^^^^^^^^^^^^^^^^^^^ Only whitespace, better avoid. -(defun vc-update-changelog-rcs2log (files) +(defun vc-default-update-changelog (backend files) "Default implementation of update-changelog. Uses `rcs2log' which only works for RCS and CVS." ;; FIXME: We (c|sh)ould add support for cvs2cl @@ -2994,7 +2926,9 @@ (mapcar (lambda (f) (file-relative-name - (expand-file-name f odefault))) + (if (file-name-absolute-p f) + f + (concat odefault f)))) files))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Is this change intentional or a merge problem? It's hard to tell... "done" (pop-to-buffer (get-buffer-create "*vc*")) (defun vc-default-rename-file (backend old new) (condition-case nil @@ -3049,8 +2989,8 @@ "Return a string with all log entries stored in BACKEND for FILE." (if (vc-find-backend-function backend 'print-log) (with-current-buffer "*vc*" - (vc-call print-log (list file)) - (vc-call wash-log file) + (vc-call print-log file) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This line is incorrect, the original is right. (defun vc-default-unregister (backend file) @@ -3119,26 +3059,6 @@ (and (not vc-make-backup-files) (delete-file backup-name)))))) (message "Checking out %s...done" file)))) -(defun vc-default-wash-log (backend file) - "Remove all non-comment information from log output. -This default implementation works for RCS logs; backends should override -it if their logs are not in RCS format." - (let ((separator (concat "^-+\nrevision [0-9.]+\ndate: .*\n" - "\\(branches: .*;\n\\)?" - "\\(\\*\\*\\* empty log message \\*\\*\\*\n\\)?"))) - (goto-char (point-max)) (forward-line -1) - (while (looking-at "=*\n") - (delete-char (- (match-end 0) (match-beginning 0))) - (forward-line -1)) - (goto-char (point-min)) - (if (looking-at "[\b\t\n\v\f\r ]+") - (delete-char (- (match-end 0) (match-beginning 0)))) - (goto-char (point-min)) - (re-search-forward separator nil t) - (delete-region (point-min) (point)) - (while (re-search-forward separator nil t) - (delete-region (match-beginning 0) (match-end 0))))) ^^^^^^^^^^^^^^^^^^^^^^^^^ What is wrong with this function? Why not keep it? @@ -3204,13 +3124,13 @@ ;; Run through this file and find the oldest and newest dates annotated. (save-excursion (goto-char (point-min)) - (while (not (eobp)) - (when (setq date (vc-call-backend vc-annotate-backend 'annotate-time)) - (if (> date newest) - (setq newest date)) - (if (< date oldest) - (setq oldest date))) - (forward-line 1))) + (while (setq date (prog1 (vc-call-backend vc-annotate-backend + 'annotate-time) + (forward-line 1))) + (if (> date newest) + (setq newest date)) + (if (< date oldest) + (setq oldest date)))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I this intentional? It seems that the CVS version might have changed after you branched your version...