VCS Interface classes

[Andreas Pakulat]

On 01.05.07 19:02:17, Matthew Woehlke wrote:
> 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:
> >> 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()?
 
Sounds a bit better, although we'd then have to apply this everywhere.

BTW: It just occured to me that our commit() doesn't have a QString
argument for the commit message ;)

> I'm still not convinced about diff()... would you be OK making that one 
> diff() and showDiff()?

Sure.

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

svn for example tells you that it doesn't diff binary files. But I'm
following you here, if there's a IBVC::cat implemented the plugin may
have this feature by doing it for certain mimetypes itself.

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

Yeap, although I hope its rather our own diff-lib (which should be
derived from kdiff3 to have 3-way diff for merging). We already have
libdiff2 from kompare in the teamplugin atm.

> IOW, diff() isn't returning "a diff", what it /really/ 
> needs is the complete copy of each item.

Yeap, I think diff returning a real diff is rather hard, unless we
settle on unified diff format - for text files - which may not play well
with some users/vcs systems.

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

Not sure I completely follow you here, but if I understood correctly
you'd like to have an option on diff (i.e. a flag) to say:

return versionA + versionB - completely
return versionA + diff to versionB
return whatever is easiest for the plugin to compute (i.e. don't care).

> Although I guess this indirectly means that cat() should move to IBasic? 

No, cat is a function that takes a remote location and IMHO we should
have that in a separate interface. I only left the QVariant in diff() in
the basic interface because thats something people will often want to
do, diff a local file against some remote version. But if a plugin
cannot do such things (i.e. vcs doesn't support directly working on the
repository) it will only provide diffing against whatever is in the
working copy (for example show changes between last update and now - svn
can do this without querying the repository).

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

I'm pretty much against this for two reasons:

a) we have a pure interface, so each plugin would have a copy of the
code - unless I misunderstood something
b) a plugin may have really optimized methods to get various types of
diffs, for example svn knows the original content of a file and thus
doesn't need to contact the repository to tell you what you changed
between the last update and now. This is important, IMHO, we shouldn't
limit the freedom of the plugin just to share some more lines of code.

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

No, and its not yet in my local file :)

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

What do you mean follow branches?

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

Thats rather logMessage(Revision) IMHO and should go into our
IRepositoryVersionControl.

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

Well, for the working copy root dir you'd get the list of files with
their respective version as result:
andreas at morpheus:~/KDE-work/4.0/kdevelop>svn stat -u -N .
?                   KDevelop4.kdevelop.filelist
?                   .kdev4
?                   svn-commit.2.tmp
?                   svn-commit.3.tmp
?                   KDevelop4.kdevses
?                   KDevelop4.kdevelop
?                   KDevelop4.tag
M          660342   kdevelop.kdev4
?                   KDevelop4.kdevelop.pcs
?                   svn-commit.tmp
Status bezogen auf Revision: 660464

For another subdir you'd get:

andreas at morpheus:~/KDE-work/4.0/kdevelop>svn stat -u -N lib
Status bezogen auf Revision: 660465

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

The only "problem" here is that number may include some arbitrary
characters, like dots. Maybe it should be more something like
FormattedNumber or .... OTOH we can just document that this needs to be
treated as a string and never tried to convert into a real number - in
the general case.

Andreas

-- 
Are you a turtle?