Andreas Pakulat wrote:
> On 30.04.07 12:06:55, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> I present to you our new VCS interface classes
(that I finally managed
>>> to write up, based on the enourmus thread some
time ago).
>>>
>>> * major problems for a given system with the
proposed interface
>> IMO IBasicVCS is missing unedit(). Jakob already
pointed out the gaping
>> flaw in commit()  .
>
> What should unedit() do? I mean, revert() reverses to
repository
> version, commit() checks in the local version into
repository. What else
> is needed?
unedit() clears the effect of edit() iff* diff(BASE,WORKING)
is empty,
i.e. the file has been marked for edit but does not actually
differ from
the repo copy.
(*"if and only if" for you non-math-types  )
>> Come to think of it, it would rock if we handled
change sets... yes,
>> this is harder to do in e.g. svn since the plug-in
would have to manage
>> it rather than being able to leverage VCS
functionality, but I know /I/
>> at least would find it really nice to have change
sets in svn.
>
> Please feel free to present an interface for that. It
shouldn't be part
> of our basic - all vcs plugins have to do this -
interface, IMHO.
I'll need to think about it for a bit. Perforce
almost has to have
this. For VCS's that don't support it, it should be possible
to write a
generic version that uses the local file system to provide
this
functionality.
>> Is VCSState::Unknown equivalent to 'not under
version control', or
>> should there be a seperate status (as I think there
is in 3.4) for
>> 'unable to determine that information'? Or does
that only need to exist
>> in the UI side?
>
> I'm not sure, what is it used for in 3.4? I just took
the 3.4 VCSState
> and removed the stuff that is not of much interest or
just doesn't map
> to all vcs'es.
It is/would be used for "hasn't been queried yet"
(esp. with an async
status()), or for things that the VCS plugin wasn't
consulted on, or
didn't return an answer... I imagine the same cases would
apply, but as
I said all of those seem to be statuses that only apply to
the UI, i.e.
the VCS plugin would never return such a status.
>> VCSMapping is insufficient for perforce which needs
exclusion as well as
>> inclusion rules. Is this just not available outside
of a private (i.e.
>> not-matching-the-interface), perforce-specific
checkout()? (Or is it in
>> the missing CliencMapping class? )
>
> ClientMapping == VCSMapping, seems I forgot one or two
places to rename
>
>
> So what you need for Perforce is something like
>
> class VCSMapping
> {
> addMapping( repoLocation, localLocation, recursive);
> addExclude( repoLocation, recursive);
> }
>
> (ommitting the other methods, like excludes() and so
on), where the
> checkout would check out all mappings, unless a mapping
or part of it
> matches an exclude? I'm fine with adding that. But I
guess the VCS
> interface then needs a function like:
>
> supportsExcludes();
>
> which tells the client wether the exclude's will be
used or not. Because
> else this is hard to do in svn plugin.
Yes, you need some sort of 'what do you support' function.
IIRC we had
originally talked about using flags for the mapping instead
of 'bool
recursive'; you would then ask for supported flags. This
would be most
extensible in case some VCS supports other things as well
as, or instead
of, exclusions.
>>> * return types for the methods, we need a way
to communicate at least
>>> errors back, but I'm not sure how to do that
best given that the
>>> methods should execute asynchronously
>> Change void to int, return 0 for success, <0 for
error (possibly allow
>> the return code to be implementation defined?). Or
have an interface to
>> retrieve an error.
>
> Actually I think 2 signals:
>
> finished( Command );
> error( Command, Message);
>
> should be fine (command is an enum for the various
methods we have).
Hmm... ok, that makes sense, but what if you issue several
calls to
status()? I think we should return something more like a pid
to use in
the signal. In fact, that should work, because then all
methods can be
made async() and we can add a wait() method. I'm not sure
how the
scripting will work but it seems that it will be more common
for scripts
to want to be synchronous because I would think they usually
rely on the
previous command to complete before they can issue the next.
I'm not
necessarily saying we should change anything, just something
to keep in
mind.
>> I'm not sure about async, I would image async in a
script is not really
>> what you want . Therefore
it seems like maybe we need both?
>
> Yeah, I thought so to. For sync methods we'd return an
error code +
> message (like QPair<int,msg>) and for async there
are the two signals.
See above; we could use task ID's and wait(tid). Scripts can
either use
this or we can somehow arrange a layer of indirection for
scripts.
>> Certainly I would guess there should be a
statusAsync() based on
>> previous discussion of svn in 3.4.
>
> Well, checkout also falls into the category of async
methods, you don't
> really want to be interrupted hacking kdevelop just
because you check
> out kdelibs where you want to do something later on.
Agreed. I never said status should be the *only* async
method. Just
that there should definately be an async status().
> I'd like to see
> mostly all things in an async way. Eric4 just gained
the possibility to
> have async commit() and that way you can initiate a
commit() and then go
> back to the IDE and look at the diff of your local
changes to write a
> proper commit message. Which IMO rocks. Not even
Eclipse has this, IIRC.
Isn't that just a matter of the pending commit() not
blocking the rest
of the interface, and the dialog being non-modal? p4v (the
perforce gui)
has been this way for I think as long as I have been using
it. Basically
until you hit 'ok' you really haven't done anything. 'svn
ci' locks the
list of files to commit but IMO we should take after p4v and
lift this
restriction, i.e. allow you to change what files are being
committed as
you are writing the message (this is one feature in p4v I
really miss in
svn, even 'p4 submit' allows you to change your mind while
in the child
editor process). p4v even updates the list of files that you
*might*
commit in real time, i.e. if you start a commit and then
edit another
file. I see no reason other plug-ins couldn't do this. (For
ones without
an explicit 'edit', it's ok if they don't always get file
change
notifications, but it's simple for edit() to add a file to
the list of
might-commit.)
>> IMO it would be ideal if we can require plug-ins to
be thread safe and
>> have a generic async wrapper around whichever
plugin. Or just allow
>> inheriting a base class that uses the virtual
synchronous methods to
>> do async and allow a plug-in to overload as
needed?
>
> Not sure I follow you here.
If we only have async methods (and a wait()) you can safely
ignore most
of that, plug-ins will have to be thread-safe anyway.
Basically, a plug-in has to support doing multiple instances
of any
combination of synchronous operations at the same time. One
way to
achieve this would be to only implement synchronous
operations that are
thread-safe and reentrant-safe, and inherit a class that
wraps the
synchronous calls around async invokers.
Does that make sense?
>> For move():
>>> * depending on the VCS, only preserves history
if copy preserves history
>> ...I thought I remember someone saying this is not
necessarily the case
>> (as nonsensical as that seems), i.e. move might
preserve history while
>> copy does not? Anyway that's just a doc nitpick.
>
> I guess we should just add "might preserve history
or not, depending on
> the VCS" to both move and copy.
Right, something like that.
>>> * Parameters for push/pull in the distributed
vcs iface, no idea whats
>>> needed there
>>> * diff and log parameters, am not 100% sure
there
>> Should diff() pop up a UI, or return something
useful in scripts?
>
> The first I think. Mainly because representing a diff
is not that easy.
> For example a diff on binary files is not possible with
svn, but might
> be with other VCS systems and then you can hardly
represent that
> in a meaningful way, except with a GUI. (I'm thinking
of KDE's commit
> digest which has visual diff for .ui files)
...but a .ui isn't a binary file . I guess
I can live with that,
though.
>>> * usage of simple QString's for any parameter
that can be a repository
>>> url (anything that is always a local url
should stay KUrl), if a VCS
>>> system can't use a url for describing a
repository location
>> ...if diff() takes a repo location shouldn't the
parameter be QString?
>
> Thats something I wasn't sure about, so I made my first
write-up easier
> by sticking to KUrl everywhere ;) The thing is I don't
know wether the
> "targeted" vcs systems (i.e. the ones we were
talking about here) really
> work with a url scheme. If not repo location needs to
turn into a
> QString and then we really need 2 different
interfaces.
Is 'svn+ssh://path/to/somewhere' a valid KUrl? (I guess if
'+' is
allowed in a protocol name and the protocol doesn't need to
be known.)
Perforce repo paths, however, look like
'//path/to/somewhere'. Of course
if you needed to do something silly like use
'p4://path/to/somewhere' in
all external code, I don't see a problem with that, the back
end can
just strip the protocol.
--
Matthew
If you believe you received this e-mail in error, you are
probably sadly
mistaken, but if not, aren't you lucky?
_______________________________________________
KDevelop-devel mailing list
KDevelop-devel kdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel
|
