VCS Interface classes

[Andreas Pakulat]

On 01.05.07 15:43:37, Matthew Woehlke wrote:
> Andreas Pakulat wrote:
> > On 01.05.07 12:40:09, Matthew Woehlke wrote:
> >> Should localRevision() have a default for the second argument? (I'm not sure if 
> >> this needs to be in the interface?) If this is to be synchronous (should it 
> >> be?), Revision needs a way to specify that an error occurred.
> > 
> > Invalid Revision should do for that, no?
> 
> What's "Invalid Revision"?

Revision.isValid() == true :) (Yes I know that method is not in the
class yet)

> > [snip] although code doesn't really have a way to find out
> > which plugin it talks to at the moment.
> 
> Um... that needs to be fixed, otherwise how can you use things not in 
> the generic interfaces? (I guess it is enough to ask if a particular 
> interface is implemented, where "particular interface" can be an 
> interface defined by a plugin, i.e. IPerforceInterface, 
> ISubversionInterface, etc.?)

Uhm, right. Somehow I was a bit in a rush (you know girlfriend wanting
to be taken care of). Thats how it would work indeed. Than any user can
check which specific interface is implemented.

> That reminds me, I /really/ would like to see log() return a standard 
> format, so we could have The One True Log Window,

We can have that window without having log() return a standard format.

> and add features like diffing two versions straight from said window.
> I think this is all going to be stuff talking about
> IBrowsingVersionControl, would it be OK to call that
> 'experimental/volatile' for now?

But I also think it might make sense to have some this feature. So how
about this:

fetchLog( localOrRepoPath, revision, limit)
logWindow( localOrRepoPath )

where revision may be 1 revision (i.e. where to start) or a range, in
which case limit would be ignored. limit would limit to the last n
entries in the log.

The return value can be something as simple as
(revision,log message, author, date) wrapped in a class.

Currently the only problem with such return values is they must be
types that are copyable and assignable to be put into a QVariant.

> > the list will make the semantics a bit more
> > complicated: If run on a dir with recursive == off, will it return a map
> > with 1 entry for the dir or will it return a map with all dirs/files in
> > that dir, or both?
> 
> What's the status of a directory, anyway? (Oh... right, in svn, that 
> means something). I guess it would give you just the directory? Also I 
> assume most UI's would never /want/ to recurse for performance reasons, 
> i.e. don't ask until you need to show the files.

Yeap, I think recursive should be false on many of our actions by
default. stat on an svn-versioned directory without recursive will give
you information about all files in that dir that have changed, are
unversioned, are in conflict and so on. Things like "needs update" needs
a communication with the repository anyway. So I guess the answer to my
question is: status() on a dir without recursive is giving you the list
of entries in that dir + their status information. The only thing left:
Do we also want the status information on the dir itself?

> >> +        // Remove Keyword, Branch, Tag... not portable
> >> +        // Remove PluginSpecific, plug-ins will define a superset of this list
> >> +        // if type is none of the above, that means plugin specific
> > 
> > As I said above currently users don't know which plugin and thus which
> > values in this enum are available.
> 
> Yeah, I'm thinking we should do like with VCSMapping and have a 
> supportedFlags() method somewhere (hmm, need to add that, or did I miss 
> it?). Then I guess we add everything to RevisionType with the caveat 
> that you have to check if it's supported. New plugins add new values 
> (not a BC issue, right?) as needed; if you want to use that plugin, you 
> have to build against the interfaces that have its stuff. Then we 
> clearly document what support is required and what is optional.
 
Actually I'm fine with supporting just the common stuff, i.e.
number+date, in the basic interface and let plugin's provide extensions
to that. I'm not sure however how the Qt flags stuff works and what it
needs to extend it, i.e. if we can keep the enum in the class or make it
part of the Kdevelop namespace.

> That said, if we go this way, I'm thinking GlobalNumer and FileNumber 
> should be required support with the well-documented caveat that asking 
> for one might give you the other (the returned version object will 
> always have the appropriate type set). So in case it matters where one 
> is supported and the other isn't, there is a way to find out.

Agreed.

Andreas

PS: I think we should have started with interfaces in the first place
instead of all the meta-discussion we had. Somehow this feels much more
productive, even though its still just you and me basically :)

-- 
You'll wish that you had done some of the hard things when they were easier
to do.