[VCS] VcsRevision

[Matthew Woehlke]

Andreas Pakulat wrote:
> On 21.05.07 10:52:24, Matthew Woehlke wrote:
>> Andreas Pakulat wrote:
>>> On 21.05.07 09:26:24, Matthew Woehlke wrote:
>>>> Creating a range is IMO not relevant, we already have setSourceRevision 
>>>> and setTargetRevision, so the answer is 'yes' as long as you have 
>>>> VcsRevisions to feed those. (Hmm, what happens though if you try to set 
>>>> one of those to a range? Should it return false, silently use an invalid 
>>>> revision instead, something else...?)
>>> If you try to create a range of ranges you should get an invalid
>>> VcsRevisions and a false return value on the setSource and setTarget
>>> functions.
>> Later conversation aside, are we agreeing to s/void/bool/ on these returns?
> 
> Yeap.

Done. I think we can also now remove RST::Valid and isValid()?

>>>> Actually, what is the use-case for a range, is it just merge()?
>>> No, its for diff() or log() (and maybe others I don't recall atm).
>> ...but diff() already takes two revisions? What does it do in log()?
> 
> In log it would allow you to fetch exactly all changes between the two
> revisions. This could - in theory - also be achieved by using the limit,
> but then you'd have to know how many changes where between the
> revisions... But as we agree on having overloads rather than ambigous
> classes we will do that.

Ok, it's just that I suspect such a limit to be at least as "optional" 
as the integer limit (perforce AFAICT does not support it)... but I 
guess that's ok. Change checked in for log(), please check it out and 
tell me if it's ok (<insert usual comment about how @param limit is 
"just a name" :-)>). Right now showLog() uses an implicit limit, ok to 
keep using that I guess?

Maybe we should instead take both limits at once, with default 
parameters for each (make the revision limit a pointer I guess, default 
= NULL)?

...which reminds me, at some point (I guess it's SC, or at least only 
minor changes, so no big rush) we need to hash out default parameter 
values where appropriate.

>>> Maybe we should rather have overloads
>>> for diff/log/<whatever can use a range of revisions> with 2 revision
>>> arguments instead of a range inside VcsRevision. That way we don't have
>>> to specially document when a range-revision can be used and when not.
>> Yes, that's exactly what I was leaning towards. It also seems to 
>> simplify things a lot not having a class that might have data or two 
>> other pointers to other instances of itself. :-)
> 
> Yes, everything I removed I agree with you.

log() should be ok now (see above). Is diff() ok as-is, or am I missing 
a use case?

-- 
Matthew
When in doubt, duct tape!