Malcolm Rowe wrote:
> On Thu, Mar 09, 2006 at 01:58:33AM +0000, Julian Foad wrote:
>
>>> {
>>> /* The 'svn diff [-r N[:M]] [TARGET[@REV]...]' case matches. */
>>>
>>> /* Here each target is a pegged object. Find out the starting
>>> and ending paths for each target. */
>>[...]
>>> /* Determine if we need to do pegged diffs. */
>>> if (opt_state->start_revision.kind != svn_opt_revision_base
>>> && opt_state->start_revision.kind != svn_opt_revision_working)
>>> pegged_diff = TRUE;
>>
>>That's wrong. In this section, according to the comment, we should assign
>>pegged_diff=TRUE unconditionally.
>
> Nope; 'svn diff foo' is handled by that case too, and that's not a pegged
> diff (though it looks like later code may may it pegged @WORKING, which
> may work anyway). I think it only wants to be pegged if either revision
> is 'non-local'.
I disagree with you. Yes, "svn diff foo" is handled by that case. That means,
"show me the difference between the base revision and the working
pseudo-revision of this object". To me, that's a "pegged diff" because I think
of "the working files" as being a revision of a sort, rather than thinking of
this as a difference between one arbitrary tree and another arbitrary tree.
I can accept that either definition would work in this case, but that's because
pegged and non-pegged diffs are the same thing at this level, it's just a
question of whether the two paths being compared are identical or different. I
don't see the logic in saying that a diff is "pegged" only if at least one
revision is non-local:
diff foo@100 foo@200
diff foo@BASE foo@200
diff foo@100 foo@WC
diff foo@BASE foo@WC
Why should we consider just the last one to be non-pegged? (Note that it will
go through to the same diff_wc_wc function in the end, whether we call it
pegged or non-pegged at this stage.)
A different way to ask the question is: given that the distinction between
pegged and non-pegged diffs seems to affect only the user interface, is there
any reason why we want a conceptual definition of it that excludes the
BASE<->WC case?
There are more bugs with pegged diffs. For example, the previous section of
svn_cl__diff() handles the "[-rN:M] --old=TGT@PEG [--new=TGT@PEG]" case, but
overwrites the "-r" revisions with the peg revisions rather than correctly
looking the targets up at their peg revisions. I'll try to have a look at that
before committing this, in case it throws more light on the best way to do
this, but I'f fairly confident that this current patch is good.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 9 14:20:59 2006