[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: changelist feature -- keep it? tweak it? scrap it?

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2007-01-17 03:01:59 CET

On 1/16/07, Karl Fogel <kfogel@red-bean.com> wrote:

> "Aha", I hear someone object, "But why make all the different clients
> implement this, when we could implement it in Subversion and save
> them the trouble / make the feature consistent / do it most efficiently /
> etc?"
> That argument doesn't apply as well to the changelist feature as it might
> to other features. Changelists are all about interface anyway -- most of
> what's involved in getting this feature right falls on the interface side of
> the code. The rest is trivial, because it just involves passing lists of files
> to existing Subversion library APIs. All those APIs already *take* lists
> of files. You want 'svn diff' to be changelist-sensitive? Well, if your
> client is changelist-conscious, it'll just pass the files of the currently-
> active changelist when it invokes the diff APIs. It's not like tons
> of potential
> client code is getting factored away by us putting this directly in Subversion;
> most of what clients would have had to write, they'd still have to write.

I think you've gone a touch off the deep end here. :-)

You're saying that the argument for factorization "doesn't apply as
well" to this feature, and therefore we should do any factorization
*at all*?

To keep track of changelists, one needs to do some bookkeeping... just
associate a name with a list of files. You're advocating that every
single client app create some sort of private database to do this?
But wait... we already *have* a database of sorts... the entries file!
 It's sitting right there, in front of every app. It's utterly
trivial to store a label in an svn_entry_t.

So that's exactly the API I wrote: (1) an API to add/remove a
changelist label from a path (by adding/removing it from the
underlying svn_entry_t). (2) an API to search the working copy for
all paths matching a changelist. Basic database functionality,
nothing more.

All the rest is svn-commandline client UI stuff, just as you
suggested. 'svn status' showing changelists? That's entirely in the
commandline client. The whole 'svn changelist blah blah' command?
Entirely in the commandline client.

The only other API tweaks I made were some optional new 'changelist'
arguments to svn_client_commit() and svn_client_revert(), so that
these existing APIs could do internal filtering on a changelist name.
But heck, if you feel strongly about it, I could undo those changes,
and just let every client application extract the changelist paths,
and then pass the whole path-list directly into these functions.
Though once again, if *every* application has to do that same task, it
makes one ask: why isn't the API doing the filtering itself? Why not
factorize? I don't think these extra arguments to commit and revert
are dictating fundamental UI decisions to applications.

I think you are vastly overestimating the complexity of the existing
code. It's nothing more than an extra field in svn_entry_t, and
teaching some svn_client_* APIs to filter on that field. The "hard"
stuff -- like UI -- already rests squarely at the application level,
exactly where you think it should be.

(Does the current commandline UI stink? Maybe? Then let's fix the
commandline client. Let's encourage TortoiseSVN to do something cool
and GUI-like. AFAICT, no client has been hindered here by the
existing code, only helped.)

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 17 03:28:59 2007

This is an archived mail posted to the Subversion Dev mailing list.