VCS Interface classes

[Andreas Pakulat]

On 02.05.07 18:19:49, Matthew Woehlke wrote:
> Andreas Pakulat wrote:
> > On 02.05.07 15:45:02, Matthew Woehlke wrote:
> >> Andreas Pakulat wrote:
> >>>> 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.
> >> It's the users I'd be worried about, we /do/ want to work out The Right 
> >> Way here.
> >>
> >> Why does it matter to the VCS? Am I missing something?
> > 
> > IIRC Kuba said with aegis he can use whatever diff tool he likes to use
> > and unified diff is certainly just one text-diff-format. Of course we
> > can settle for that - its pretty wide-spread in unix world. The thing
> > I'd like to emphasize here is that we don't try to support more than one
> > format - if we allow to extract a textual diff.
> 
> Ok, I can live with that. I would say if aegis wants to get fancy here, 
> let it, that's what per-plugin stuff is for. Otherwise I prefer being 
> able to write a generic showDiff that lets *all* VCS's take advantage of 
> being able to diff binary files in a by-mimetype manner.

If this is all about showDiff all we really need is a diff-widget which
can use whatever format we want to use (probably unified diff) and the
plugins then use this widget. Thats as much code-reuse as you can get
for showDiff. 

> > Last but not least:
> > How can the generic implementation work if it can't subclass from
> > QObject [snip]
> 
> #define MY_CLASS_NAME SvnPlugin
> #include "generic-showdiff-impl.cpp"
> :-)

Do I have to write it down, what I think about his or is a simple no
enough? ;)

> > Ugly as hell - IMHO. Let the plugins implement diff the way they want
> > it, you simply can't get everything under one hood.
> 
> See above. But, just to be clear, I'm talking about /showDiff()/, not 
> diff().

See further above. I thought you were talking about normal diff(). If
this is about code-reuse for showDiff so the plugins don't invent their
own machinery for showing a unified diff then we're totally on the same
page with this (not the cat-move though).

> >> The opposite of 'svn log --stop-on-copy' :-). (Sorry, perforce 
> >> terminology slipping in again.) In perforce you have to specifically 
> >> request this, although I think (like svn) it should be default. I was 
> >> just wondering if you think there should be an option to disable this?
> > 
> > In a GUI maybe, not via scripts, too much a border case - IMHO.
> 
> Maybe. I'm not arguing for it either way, I'd be fine making 'follow' 
> the default and non-optional. Although I do wonder if log() should 
> return the repo path at each revision, otherwise can you ask about an 
> old version with a different name? (I know svn doesn't give file name by 
> default, and frankly, it drives me nuts, because you don't know what's 
> happening. IMO log() should tell you what repo path each entry is 
> talking about. :-))

Well, we can change the return value of log easily to this:
andreas at morpheus:~/KDE-work/4.0/kdevelop/lib/plugins/vcs/subversion>svn log -v --limit 2 .
------------------------------------------------------------------------
r658972 | scripty | 2007-04-29 07:01:30 +0200 (So, 29 Apr 2007) | 1 line
Geänderte Pfade:
   M /trunk/KDE/kdevelop/lib/plugins/vcs/cvs/kdevcvs.desktop
   M /trunk/KDE/kdevelop/lib/plugins/vcs/subversion/kdevsubversion.desktop
   M /trunk/KDE/kdevelop/lib/shell/settings/kcm_kdev_uisettings.desktop
   M /trunk/KDE/kdevelop/plugins/teamwork/kdevteamwork.desktop

SVN_SILENT made messages (.desktop file)
------------------------------------------------------------------------
r658112 | apaku | 2007-04-26 01:30:17 +0200 (Do, 26 Apr 2007) | 3 lines
Geänderte Pfade:
   M /trunk/KDE/kdevelop/cmake/modules/FindCommoncpp.cmake
   M /trunk/KDE/kdevelop/cmake/modules/FindSubversionLibrary.cmake
   M /trunk/KDE/kdevelop/lib/plugins/vcs/CMakeLists.txt
   M /trunk/KDE/kdevelop/lib/plugins/vcs/subversion/CMakeLists.txt

Make the library finders a bit more quiet
Add new variable to FindSubversion which contains all include dirs for svn,apr and apu

------------------------------------------------------------------------

Is that more interesting?

> >>>> Ok, addition: IFooVersionControl (AtomicBrowsing?):
> >>>> revision(Revision) // gets information about a specified revision.
> >>> Thats rather logMessage(Revision) IMHO
> >> ...logMessage() sounds to me like a subset, not a superset, of log() 
> >> (referring to what information you get back) :-). Not that I'm really 
> >> crazy about revision() either, to be honest.
> > 
> > Maybe I misunderstood but IIRC you wanted to get the same information as
> > log has just for a specific revision right?
> 
> Yes and no. You get the user, date, etc. You /also/ get a list of what 
> files the change affected, and preferably in what manner (i.e. 
> add/del/edit). The whole point is to find out 'what did XX revision 
> change?'.

see above, we can certainly do something like

log( path, revision, fullinfo/shortinfo)

svn even shows moves in verbose mode (not sure about copies):
------------------------------------------------------------------------
r646828 | coolo | 2007-03-26 20:54:42 +0200 (Mo, 26 Mär 2007) | 2 lines
Geänderte Pfade:
   A /branches/KDE/3.5/kdevelop (von /branches/kdevelop/3.4:646827)
   D /branches/kdevelop/3.4

kdevelop 3.4 is now part of KDE 3.5


> >> ...but do repos with non-global versioning support it? [snip]
> > 
> > I think I really misunderstood something,  this doesn't make the
> > slightest sense. Of course with cvs you can get the log message of a
> > specific revision.
> > 
> > Or do you aim at getting the list of files modified in that revision? In
> > that case thats not possible with CVS I think. I don't have a cvs
> > project at hand to try what log returns.
> 
> Yes, that. :-) IMO this is a really important thing to be able to do 
> (for VCS's that support it). Hmm... /does/ svn support it? I can't 
> imagine not, but I can't figure out offhand how to do it, either (diff 
> the whole repo, maybe?).

svn log -v (the -v is important).

> >> (Also, does this mean there is no way to ask about
> >> /just/ a directory? Is that desirable?)
> > 
> > Not sure what you mean but I do get status information on just a
> > directory inside a working copy:
> 
> I meant how would you do this with the interface. Behave differently 
> based on the presence/absence of a trailing '/'? I guess that would 
> works. :-)

I'd like to avoid such things, i.e. different behaviour wether a
trailing slash exists or not. How about this: 

status( dir, nonrecursive ) returns a map of all files/dirs in that dir
with their status + 1 status for the dir itself

status( file, nonrecursive ) returns a map with 1 entry with just the
status of the file.

Thats what I'd expect, so if I give it the working copy its easy for
svn, if I give svn a dir inside the working copy svn has to do some more
svn status calls then (or reduce the return value of a recursive call).
Not a big deal.

Andreas

-- 
All the troubles you have will pass away very quickly.