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: VC command for showing outgoing changes Date: Sat, 05 Dec 2009 15:53:03 -0500 Message-ID: References: <200910132024.n9DKOHGj015040@godzilla.ics.uci.edu> <200912051945.nB5Jjam5020140@godzilla.ics.uci.edu> <200912051952.nB5JqXSm020185@godzilla.ics.uci.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1260046405 2212 80.91.229.12 (5 Dec 2009 20:53:25 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 5 Dec 2009 20:53:25 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dan Nicolaescu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Dec 05 21:53:15 2009 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 1NH1cw-0003xT-QN for ged-emacs-devel@m.gmane.org; Sat, 05 Dec 2009 21:53:15 +0100 Original-Received: from localhost ([127.0.0.1]:52210 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NH1cw-0007iY-Ky for ged-emacs-devel@m.gmane.org; Sat, 05 Dec 2009 15:53:14 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NH1cs-0007iT-O2 for emacs-devel@gnu.org; Sat, 05 Dec 2009 15:53:10 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NH1cn-0007i0-QZ for emacs-devel@gnu.org; Sat, 05 Dec 2009 15:53:10 -0500 Original-Received: from [199.232.76.173] (port=57382 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NH1cn-0007hx-OJ for emacs-devel@gnu.org; Sat, 05 Dec 2009 15:53:05 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:61509 helo=ironport2-out.pppoe.ca) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NH1cn-0008Vj-DN for emacs-devel@gnu.org; Sat, 05 Dec 2009 15:53:05 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Au4EADdXGktFpZ7i/2dsb2JhbACBS9J3hDMEih6DBg X-IronPort-AV: E=Sophos;i="4.47,347,1257138000"; d="scan'208";a="50758584" Original-Received: from 69-165-158-226.dsl.teksavvy.com (HELO pastel.home) ([69.165.158.226]) by ironport2-out.pppoe.ca with ESMTP; 05 Dec 2009 15:53:04 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id B876580B1; Sat, 5 Dec 2009 15:53:03 -0500 (EST) In-Reply-To: <200912051952.nB5JqXSm020185@godzilla.ics.uci.edu> (Dan Nicolaescu's message of "Sat, 5 Dec 2009 11:52:33 -0800 (PST)") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. 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:118323 Archived-At: >> Here's a patch that implements the generic vc-incoming, vc-outgoing and >> implements the backend specific functions for bzr, hg and partially for >> git (no incoming and outgoing is not quite right). I'd call them vc-log-incoming and vc-log-outgoing, to make it clear that it shows this info in the form of a changelog (we could also have vc-diff-incoming, vc-diff-outgoing, for example). [ Another option might be to specify the `outgoing' or `incoming' as arguments to vc-print-log or vc-diff. Basically your `vc-(in|out)going' is like (vc-print-log from-upstream to working-revision). ] >> OK to check in? Not quite, see below. > + (when (boundp 'vc-log-view-type) > + (set (make-local-variable 'log-view-type) vc-log-view-type)) Yuck. Please avoid dynamic scoping when possible. E.g. you could do it simply via: > +(defun vc-incoming-internal (backend remote-location) > + (let ((buff-name "*vc-incoming*")) > + (vc-call-backend backend 'incoming buff-name remote-location) > + (pop-to-buffer buff-name) > + (vc-exec-after > + `(let ((inhibit-read-only t)) > + (vc-call-backend ',backend 'log-view-mode) (set (make-local-variable 'log-view-type) 'incoming) > + (set (make-local-variable 'log-view-vc-backend) ',backend) > + (set (make-local-variable 'log-view-vc-fileset) nil) > + (shrink-window-if-larger-than-buffer) > + (setq vc-sentinel-movepoint (point)) > + (set-buffer-modified-p nil))))) > + > +(defun vc-outgoing-internal (backend remote-location) > + (let ((buff-name "*vc-outgoing*")) > + (vc-call-backend backend 'outgoing buff-name remote-location) > + (pop-to-buffer buff-name) > + (vc-exec-after > + `(let ((inhibit-read-only t) > + (vc-log-view-type 'outgoing)) > + (vc-call-backend ',backend 'log-view-mode) > + (set (make-local-variable 'log-view-vc-backend) ',backend) > + (set (make-local-variable 'log-view-vc-fileset) nil) > + (shrink-window-if-larger-than-buffer) > + (setq vc-sentinel-movepoint (point)) > + (set-buffer-modified-p nil))))) The redundancy between these two functions is bad. Worse: there's redundancy between this duplicate code and vc-print-log-internal as well, some of what is different is a bug: (vc-call-backend ',backend 'log-view-mode) should be called before `vc-exec-after', as seen in vc-print-log-internal. Also incoming/outgoing will probably want to obey vc-log-short-style, so we really want to use as much of vc-print-log-internal as possible here. > -(defvar vc-short-log) > +(defvar log-view-type) > (define-derived-mode vc-hg-log-view-mode log-view-mode "Hg-Log-View" > (require 'add-log) ;; we need the add-log faces > (set (make-local-variable 'log-view-file-re) "\\`a\\`") > (set (make-local-variable 'log-view-per-file-logs) nil) > (set (make-local-variable 'log-view-message-re) > - (if vc-short-log > + (if (eq log-view-type 'short) > "^\\([0-9]+\\)\\(?:\\[.*\\]\\)? +\\([0-9a-z]\\{12\\}\\) +\\(\\(?:[0-9]+\\)-\\(?:[0-9]+\\)-\\(?:[0-9]+\\) \\(?:[0-9]+\\):\\(?:[0-9]+\\) \\(?:[-+0-9]+\\)\\) +\\(.*\\)$" Ah, so that's why you use dynamic-scoping for log-view-type, so it's available while setting up the major-mode. Hmm... And there's another related problem: > + (if (memq log-view-type '(short outgoing)) This is becoming very ad-hoc. I think the right answer is to let the print-log backend operation set something up that the log-view-mode operation can use subsequently. I.e. remove `log-view-mode' from the generic part of the code, and let the backend set some permanent-local variable in `print-log' to tell the subsequent `log-view-mode' which kind of log to expect. We could still define a generic `log-view-type' or `log-view-format' variable for it, tho. The idea is that the generic part of the code may want to use it, e.g. to display it in the mode-line (and let button-2 run a command that changes the format to something else). But it should be set by the backend's `print-log' operation rather than by the generic code. WDYT? Stefan