On Mon, Jul 13, 2009 at 5:39 PM, Stephen Leake
<address@hidden> wrote:
I'd like to add a '--reverse' option to diff.
...
if (app.opts.reverse)
{
// FIXME: this breaks something in graph.cc
make_cset(new_roster, restricted_roster, excluded);
make_cset(restricted_roster, old_roster, included);
}
else
{
make_cset(old_roster, restricted_roster, included);
make_cset(restricted_roster, new_roster, excluded);
}
But this breaks an invariant in graph.cc when app.opts.reverse is
true.
I have two questions:
1) 'excluded' is a local variable; it is _not_ returned to the caller.
'included' is returned to the caller. Yet it seems that 'excluded'
contains the diff they are interested in! How does this work? Why are
There are three rosters involved: (1) the old_roster which is the one you're diffing from, often your workspace base roster; (2) the new_roster which is the one you're diffing to, generally the unrestricted content of your modified workspace; and (3) the restricted_roster, which is some "distance" between the old and new roster, depending on what files have been included in the restriction. If there is no restriction specified then all changes are included and restricted_roster == new_roster. If an overly restrictive restriction is specified then no changes are included and old_roster == restricted_roster.
The included cset contains the changes from the old_roster to the restricted_roster (those that are included by the restriction). The excluded cset contains the changes from the restricted_roster to the new_roster (those that are excluded by the restriction).
The reverse diff you are looking for is the one from the new_roster back to the restricted_roster. The remaining diff from the restricted_roster back to the old_roster will contain the changes excluded by any restriction. I think you just need to swap included/excluded in you cset construction.
The comments in restrictions.hh might help. Also, although revert doesn't actually use a cset to do the real work, there are some comments in there describing what should happen if it did. If you can get your head around what that would do you should have it figured out! ;)
we building two csets, but only returning one of them?
From what I can see the excluded cset is never used, calculating it is completely redundant and should be removed.
2) Any suggestions for fixing this? I'll investigate graph.cc myself,
but I'm hoping someone has some pertinent advice.
A complete guess but the "new_is_archived" flag is probably used to pull the new revision_from the database in the case when it lives there. When you're going in reverse "new" probably has a different meaning. If your workspace is uncommitted and you're attempting to diff from some committed revision *back* to your uncommitted workspace then "new" really means "old" (i.e. your workspace) and new_is_archived should be false.
Hope this helps.
Cheers,
Derek