Re: VCS Interfaces, round 3

Andreas Pakulat wrote:
> On 04.05.07 13:43:22, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> On 04.05.07 11:24:46, Matthew Woehlke wrote:
>>>> ...which still leaves the question of how
do you specify the limit?
>>> Simple unsigned int. If its 0 fetch all
revisions, any number larger
>>> than 0: Fetch that many revisions from the
repository.
>> Hmm, I guess no one will have more than four
billion revisions?  
>> (Given that long-lived products seem to be in the
hundred-thousands 
>> range that is probably safe until KDE 5 at least
.)
> 
> Well, we can make it an unsigned 64bit number, I doubt
anybody will have
> that many revisions. I was just saying that its a plain
number, not a
> QString or some such as it really is just something for
"counting".

Right. I know saying "32 bits is enough for
anyone" is one of those 
famous-last-words things, but in this case it only needs to
be enough 
until KDE5. 
Alternatively, if someone wants to limit the log to more 
than four billion, I don't feel so bad telling them that
they can't do 
that. 

>>>> Come to think about it, it would be ideal
if VcsJob provided a way to 
>>>> return results incrementally.
>>> We can follow QProcess here and emit
resultsReady() then the user of the
>>> job can fetch the results and finished() would
be emitted when the job
>>> is completely done.
>> Sounds good. I assume resultsReady() is fired 0 or
more times? (I.e. 
>> keep firing it at results come in, possibly
rate-limited? The rate limit 
>> would also be nice if the operation completes
really fast, you would 
>> skip right to finished().)
> 
> No, resultsReady would always emit at least once,
unless there are no
> results on an action (think add, commit and so on).

So it is (potentially) redundant with finished()? Ok, I can
live with 
that. 

> However there we have a problem, when the job is really
fast we run into
> trouble because it might finish before the signals are
connected. So
> should we maybe have a "start" slot in the
VcsJob to initiate the start
> from the "user"?

Good catch, we'd have that no matter what, wouldn't we?
start() would be 
fine, I'm not sure it should actually "start" the
job, just flush any 
pending signals (i.e. the job would run right away but queue
signals 
until start() is called). Or rather, it should be /allowed/
to do that, 
depending on which way the plugin finds easier.

>>> It might however mean that the backend still
>>> executes completely, just that no resultsReady
signal is emitted and the
>>> finished signal not either (rather a
cancelled() one)
>> That's fine. I imagine process-based
implementations will just SIGINT 
>> the child in this instance.
> 
> Or sigkill 

True. 
I guess I'm too polite to SIGKILL my children when it is 
generally enough to SIGINT them. :-D

>> Do we really need a canceled() signal?
> 
> Hmm, well that depends. Does the user care about the
job after having
> cancelled it? That introduces a new question: Who takes
care of deleting
> the job instance when its not needed anymore - the
user? the plugin? I
> think the user needs to do this and then cancelled is
needed, you can't
> just delete the job after calling cancel() else there
might be uncleaned
> resources...

Hmm... ok, that makes sense, I guess it may be that the job
can't be 
canceled, i.e. a synchronous cancel() would block for a
while.

-- 
Matthew
Current geek index: 62%


_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 04.05.07 15:12:21, Matthew Woehlke wrote:
> Andreas Pakulat wrote:
> > On 04.05.07 13:43:22, Matthew Woehlke wrote:
> >> Andreas Pakulat wrote:
> >>> On 04.05.07 11:24:46, Matthew Woehlke
wrote:
> >>>> Come to think about it, it would be
ideal if VcsJob provided a way to 
> >>>> return results incrementally.
> >>> We can follow QProcess here and emit
resultsReady() then the user of the
> >>> job can fetch the results and finished()
would be emitted when the job
> >>> is completely done.
> >> Sounds good. I assume resultsReady() is fired
0 or more times? (I.e. 
> >> keep firing it at results come in, possibly
rate-limited? The rate limit 
> >> would also be nice if the operation completes
really fast, you would 
> >> skip right to finished().)
> > 
> > No, resultsReady would always emit at least once,
unless there are no
> > results on an action (think add, commit and so
on).
> 
> So it is (potentially) redundant with finished()? Ok, I
can live with 
> that. 

Uhm no. I'd reduce finished() to not carry any result info
at all. I'm
thinking about this workflow:

VcsJob* job = iface->log(KUrl("somehwere"));
connect(job,SIGNAL(resultsReady()), this,
SLOT(slotResultReady()));
connect(job,SIGNAL(finished()), this,
SLOT(slotFinished()));
job->start();

slotResultReady()
VcsLog = job->result().value<VcsLog>();

slotFinished()
delete job;

Most probably with a VcsJob* parameter in the signals to
make it easier
for the using code to connect and know which job emitted the
signal.

> > However there we have a problem, when the job is
really fast we run into
> > trouble because it might finish before the signals
are connected. So
> > should we maybe have a "start" slot in
the VcsJob to initiate the start
> > from the "user"?
> 
> Good catch, we'd have that no matter what, wouldn't we?
start() would be 
> fine, I'm not sure it should actually "start"
the job, just flush any 
> pending signals (i.e. the job would run right away but
queue signals 
> until start() is called).

That would make the job class quite more complex, i.e. it
always has to
check wether "isStarted" is set to know wether a
resultsReady() or
finished() signal can be sent. IMHO start() should rather
start the job
and thus the job implementation would be much lighter and
easier.

> Or rather, it should be /allowed/ to do that, depending
on which way
> the plugin finds easier.

Hmm, I guess the above doesn't really matter for the user of
the API, so
yeah. Let the plugins decide what they want to do. The
important thing
to document is that no signal should be emitted until
start() has been
called, but they may be emitted from inside start().

> >> Do we really need a canceled() signal?
> > 
> > Hmm, well that depends. Does the user care about
the job after having
> > cancelled it? That introduces a new question: Who
takes care of deleting
> > the job instance when its not needed anymore - the
user? the plugin? I
> > think the user needs to do this and then cancelled
is needed, you can't
> > just delete the job after calling cancel() else
there might be uncleaned
> > resources...
> 
> Hmm... ok, that makes sense, I guess it may be that the
job can't be 
> canceled, i.e. a synchronous cancel() would block for a
while.

As I said to have a nice flow and a common point to cleanup
point in
user-code we should emit 3 signals when the job is done,
that is:

a) finished(VcsJob*) - the job has finished sucessfully and
all results
are available (resultsReady signals haven been emitted
accordingly).
b) error(VcsJob*) - an error occured
c) cancelled(VcsJob*) - the job was cancelled and is done
with any
cleanup it had to do due to the cancelling

Andreas

-- 
Try the Moo Shu Pork.  It is especially good today.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel