VCS Interface classes

[Matthew Woehlke]

Andreas Pakulat wrote:
> On 01.05.07 15:43:37, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> On 01.05.07 12:40:09, Matthew Woehlke wrote:
>>> [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.

Ok, sounds like a plan. :-)

>> 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 )

I've been thinking about that, too; certain methods I think it makes 
sense to have a UI and non-UI version.

I'm not really crazy about the names (but then, they're just names :-)), 
what do you think about log() and showLog()?

I'm still not convinced about diff()... would you be OK making that one 
diff() and showDiff()? For VCS's that don't support binary diff, you 
could provide such a thing in the plugin if it supports IBVC::cat, which 
would allow fancy diffs independent of the VCS. Otherwise just document 
it as allowed to fail (what would it do normally on binary files if the 
VCS doesn't support binary diff?).

Um... and we're going about this the wrong way, I think. How many 
instances need /just/ the diff, anyway? Scripted, *maybe*, but I would 
think showDiff() is really going to pop up the two versions in e.g. 
Kompare, no? IOW, diff() isn't returning "a diff", what it /really/ 
needs is the complete copy of each item. Now, it is more efficient to 
fetch /one/ copy and the patch that generates the other, but the end 
result is that you have a copy of both items. IOW format standardization 
is a non-issue because we can always generate both items, and from there 
translate that into any standard we want. (We might want to pass to 
diff() if we need both items, or item A and a diff, or if we don't care, 
so that we don't translate the hard way from one when we actually want 
the other. That would allow us to ask for 'pure' as well if we really 
/don't/ care what we started with, just the changes.)

Although I guess this indirectly means that cat() should move to IBasic? 
(And... if cat() moves to IBasic, we can have a generic diff() 
implementation... Yes, in case you hadn't noticed, I'm maniacal about 
code reuse. :-))

> 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.

Ack, that wasn't in the previous versions of the interface, was it? 
Limits are definitely good; double-plus agreed. :-)

Hmm... it occurs to me that you need this if for some reason you want to 
diff X:HEAD against the same file five revisions ago.

It also occurs to me, I guess this would follow branches by default (if 
supported by the VCS)? Or should that be an option?

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

That's what I was thinking. That sounds like a 100% match for both svn 
and perforce, probably pretty standard. Anyway I guess it can be a base 
class also (i.e. a plugin can subclass if it wants), so you don't lose 
anything.

Ok, addition: IFooVersionControl (AtomicBrowsing?):
revision(Revision) // gets information about a specified revision.

...i.e. cvs would not implement this interface (I think?). The return 
would be the same as log, subclassing the return class and adding a list 
of affected paths.

>>> 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?

Um, actually I was assuming status told you about exactly what you asked 
about, i.e. if you want to know about files 1 level deep, you give in 
the list files 1 level deep. Otherwise it gives you one result; the 
directory. :-)

And in general, yes I think we should return a result on the directory. 
A question; should a directory 'need updating' if any file within needs 
updating? Also, for VCS's that don't version directories, I guess it 
should return Unknown if there are no versioned files in the directory, 
otherwise treat it as existing (and deleted, if all versioned files 
within are marked for delete).

> 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.

Ok, I'm fine with that too (as long as it works). :-) I'd probably 
prefer it; less confusion about what is supported.

> 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 :)

Of course it feels more productive, we have a tangible product now. :-)

The real question is, would we be moving forward so fast/well with the 
implementation if we hadn't discussed it first? Who can know? :-)

-- 
Matthew
"Will somebody get this walking carpet out of my way?!"
   -- Princess Leia Organa