[PATCH V2] localrepo: reverse contexts in status

[Sean Farley]

Pierre-Yves David <pierre-yves.david at ens-lyon.org> writes:

> On 05/15/2014 06:09 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley at gmail.com>
>> # Date 1397684088 18000
>> #      Wed Apr 16 16:34:48 2014 -0500
>> # Node ID b96074a56e0d75b39476e37d91395505b1e01d2f
>> # Parent  8844e50911049ffb10c3693b6cfea7e2ea4440c7
>> localrepo: reverse contexts in status
>>
>> This is a slight tweak to how localrepo.status calculates what files have
>> changed. By forcing a changectx to be first operator and anything not a
>> changectx to be the second operator, we can later exploit this to allow
>> refactoring the status operation as a method of a context object.
>>
>> Furthermore, this change will speed up 'hg diff --reverse' when used with the
>> working directory because the code will now hit a fast path without needing to
>> calculate an unneeded second manifest.
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -1521,10 +1521,28 @@ class localrepository(object):
>>               return mf
>>
>>           ctx1 = self[node1]
>>           ctx2 = self[node2]
>>
>> +        # This next code block is, admittedly, fragile logic that tests for
>> +        # reversing the contexts and wouldn't need to exist if it weren't for
>> +        # the fast (and common) code path of comparing the working directory
>> +        # with its first parent.
>> +        #
>> +        # What we're aiming for here is the ability to call:
>> +        #
>> +        # workingctx.status(parentctx)
>> +        #
>> +        # If we always built the manifest for each context and compared those,
>> +        # then we'd be done. But the special case of the above call means we
>> +        # just copy the manifest of the parent.
>> +        reversed = False
>> +        if (not isinstance(ctx1, context.changectx)
>> +            and isinstance(ctx2, context.changectx)):
>
> Not a super fan of isinstance. Do we have any other alternative?
> (no is a valid answer)

This is just a stop gap until we can figure out something
better. Especially when memctx is thrown into the mix. 'isinstance' will
keep things working even when memctx will soon inherit from
committablectx. After that we could discuss how to approach it (just
like we could discuss the pre/post status methods at that time).

>> +            reversed = True
>> +            ctx1, ctx2 = ctx2, ctx1
>> +
>>           working = ctx2.rev() is None
>>           parentworking = working and ctx1 == self['.']
>>           match = match or matchmod.always(self.root, self.getcwd())
>>           listignored, listclean, listunknown = ignored, clean, unknown
>>
>> @@ -1579,10 +1597,13 @@ class localrepository(object):
>>               removed = mf1.keys()
>>
>>           if working:
>>               modified = ctx2._filtersuspectsymlink(modified)
>>
>> +        if reversed:
>> +            added, removed = removed, added
>> +
>
> I see that the deleted/unknown thing disapeared, I assume it did not 
> made much sense in the first place?

I found a counterexample for switching deleted/unknown so I removed
that. If we could define this all in terms of operators and rings,
that'd be great :-)