From: Dan Nicolaescu <dann@ics.uci.edu>
To: esr@thyrsus.com
Cc: emacs-devel@gnu.org
Subject: Re: New VC mode -- review request
Date: Wed, 03 Oct 2007 07:31:35 -0700 [thread overview]
Message-ID: <200710031431.l93EVfFZ021233@oogie-boogie.ics.uci.edu> (raw)
In-Reply-To: <20071003103501.GA4997@thyrsus.com> (Eric S. Raymond's message of "Wed\, 3 Oct 2007 06\:35\:02 -0400")
"Eric S. Raymond" <esr@thyrsus.com> 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
next prev parent reply other threads:[~2007-10-03 14:31 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-03 10:35 New VC mode -- review request Eric S. Raymond
2007-10-03 14:31 ` Dan Nicolaescu [this message]
2007-10-03 17:00 ` Eric S. Raymond
2007-10-04 0:42 ` Dan Nicolaescu
2007-10-04 2:02 ` Eric S. Raymond
2007-10-04 2:14 ` Dan Nicolaescu
2007-10-09 14:55 ` Eric S. Raymond
2007-10-09 15:33 ` Dan Nicolaescu
2007-10-09 17:21 ` Eric S. Raymond
2007-10-04 7:18 ` Thien-Thi Nguyen
2007-10-03 21:32 ` Alexandru Harsanyi
2007-10-04 2:24 ` Dan Nicolaescu
2007-10-04 9:46 ` Alexandru Harsanyi
2007-10-04 9:51 ` Eli Zaretskii
2007-10-04 17:27 ` Richard Stallman
2007-10-04 20:32 ` David Kastrup
2007-10-04 2:02 ` Richard Stallman
2007-10-04 2:22 ` Eric S. Raymond
2007-10-05 16:12 ` Richard Stallman
-- strict thread matches above, loose matches on Subject: below --
2007-10-05 12:15 Paolo Bonzini
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=200710031431.l93EVfFZ021233@oogie-boogie.ics.uci.edu \
--to=dann@ics.uci.edu \
--cc=emacs-devel@gnu.org \
--cc=esr@thyrsus.com \
/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.