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 mode -- review request Date: Wed, 03 Oct 2007 07:31:35 -0700 Message-ID: <200710031431.l93EVfFZ021233@oogie-boogie.ics.uci.edu> References: <20071003103501.GA4997@thyrsus.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1191422054 13606 80.91.229.12 (3 Oct 2007 14:34:14 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Wed, 3 Oct 2007 14:34:14 +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 Wed Oct 03 16:34:10 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 1Id5IV-0003ZU-3G for ged-emacs-devel@m.gmane.org; Wed, 03 Oct 2007 16:33:59 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Id5IQ-0008H0-Mm for ged-emacs-devel@m.gmane.org; Wed, 03 Oct 2007 10:33:54 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Id5IK-0008DW-RR for emacs-devel@gnu.org; Wed, 03 Oct 2007 10:33:49 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Id5IK-0008CS-5t for emacs-devel@gnu.org; Wed, 03 Oct 2007 10:33:48 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Id5IJ-0008Bp-KH for emacs-devel@gnu.org; Wed, 03 Oct 2007 10:33:47 -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 1Id5IJ-0004Pk-5k for emacs-devel@gnu.org; Wed, 03 Oct 2007 10:33:47 -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 l93EVfFZ021233; Wed, 3 Oct 2007 07:31:42 -0700 (PDT) In-Reply-To: <20071003103501.GA4997@thyrsus.com> (Eric S. Raymond's message of "Wed\, 3 Oct 2007 06\:35\:02 -0400") Original-Lines: 123 X-ICS-MailScanner: Found to be clean X-ICS-MailScanner-SpamCheck: not spam, SpamAssassin (score=0.36, required 5, autolearn=disabled, ALL_TRUSTED -1.44, J_CHICKENPOX_22 0.60, J_CHICKENPOX_32 0.60, J_CHICKENPOX_42 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:80187 Archived-At: "Eric S. Raymond" writes: > I am the original author of the VC mode shipped with Emacs, back in 1992-1993. > Its design was well matched to the file-oriented version-control systems > (VCSes) of its day, and it has proven a useful tool. But it has long been in > need of a rewrite, for two reasons: > > (1) The code has accreted cruft over the years, and > > (2) the design is poorly matched to modern changeset-oriented VCSes, > beginning with Subversion and including third-generation systems such > as Mercurial and Bazaar. Thank you for working doing this, such features are sorely needed now. > Now I think it has come time for the front end to be merged. I have > been using it in my everyday development work with Subversion for months. > Stefan asked me to submit it to this list for final review, and I am > doing so. Please find it attached. The vc.el file in CVS trunk has changed since your last commit, some new features have been added, bugs have been fixed. Could you please merge your changes with the top of the trunk? > Here are the user-visible changes you should expect relative to what's > in the manual: > > * The following commands now operate on filesets rather than files. > > vc-next-action = C-x v v > vc-diff = C-x v = > vc-print-log = C-x v l > vc-revert = C-x v u This does not seem to work for me from vc-dired. I have 2 changed files in a Mercurial. After marking both of them in vc-dired and pressing v u I get this error "Please kill or save all modified buffers before reverting". None of the files is in an emacs buffer. > vc-rollback = C-x v c Do you have an implementation for this function for any backend? It would be interesting to see it. > Changes other than these are probably bugs and you should report them. You introduced "focus version". This looks like a new term, could we use something that is already in use in at least one version control system? > You need not restrict criticism to outright bugs; the user interface hasn't > been reviewed or refreshed in a very long time before this, and it is > possible we could be doing things more gracefully. You have moved functions around in the file. This makes doing a diff between the top of the trunk and your version harder than it needs to be. Could you please restore the function order (at least for review purposes) and repost? In vc-deduce-fileset: This: (let ((regexp (dired-marker-regexp)) (marked '())) (save-excursion (goto-char (point-min)) (while (re-search-forward regexp nil t) (if (setq filename (dired-get-filename nil t)) (setq marked (append marked (list filename)))))) Can be replaced with this: (dired-map-over-marks (dired-get-filename) nil)) Also: ((and vc-parent-buffer (buffer-file-name vc-parent-buffer)) (progn (if vc-dired-mode ^^^^^^^^^^^^^^ Can this ever be true, there's a previous vc-dired-mode clause in the `cond'... This code in vc-dired-hook: ;; ordinary file ((and (vc-backend filename) (not (and vc-dired-terse-mode (vc-up-to-date-p filename)))) (vc-dired-reformat-line (vc-call dired-state-info filename)) (forward-line 1)) Is the vc-backend call necessary? It ends up calling vc-registered and that can be expensive for the backends that run a program to determine the registered state. The -dir-state method computes the state for most (all?) files... I don't understand this code very well, but if it can be simplified, it might result in a significant speedup for vc-dired. It seems that you want to keep vc-dired. IMHO vc-dired is not very good because - it is slow because of running too many vc commands to find out the status for each file. - it does not show non-registered files, so one can't select them and register them - most of the dired commands are not useful in the VC context, they are just clutter... - and interface like PCL-CVS, psvn.el or git.el seems that is targeted only for version control seems better. But this is not necessarily related to your changes and should not preclude you from checking in. Do you have any plans for updating the branch infrastructure? VC now uses the term "snapshot" for branches, most people have no idea what that means... Also for people that have not used RCS "locked" is not a familiar term, would it be possible to minimize it's use? I hope this helps. Thanks! --dan