[PATCH 1 of 7] context.status: remove overriding in workingctx

[Sean Farley]

Martin von Zweigbergk writes:

> On Mon Nov 03 2014 at 10:34:50 AM Sean Farley <sean.michael.farley at gmail.com>
> wrote:
>
>>
>> Martin von Zweigbergk writes:
>>
>> > The workingctx method simply calls the super method. The only effect
>> > it has is that it uses a different default argument for the 'other'
>> > argument. The only in-tree caller is patch.diff, which always passes
>> > an argument to the method, so it should be safe to remove the
>> > overriding. Having the default argument depend on the type seems
>> > rather dangerous anyway.
>>
>> The only in-tree caller might be patch.diff but there were some
>> extensions that used it also.
>
>
> What I don't like is that ctx.status() might be comparing to either
> repo['.'] or repo[None] depending on what type ctx has. I don't know if we
> have code where the 'ctx' could be either type of context, but it just
> feels a little weird. I don't care that much, so if we want to keep it for
> BC, that's fine.

I completely agree. It feels very out-of-place that repo[None].status()
caches the status of the working directory against its parent but that
repo[None].status(REV) would calculate another status result all
together.

>
>> The design constraint was that
>> repo[None].status() had to maintain its fast path for checking the
>> dirstate and disk.
>>
>
> I don't believe I'm changing that. Am I?

No, I just put it there as reference for some of my choices. There's an
overarching goal I was trying to go towards: in-memory merge ... which
required removing the working ctx code from 'status'. This status
refactoring into context was a bit out of the way but still needed to be
done. Therefore, I wanted it to refactor but leave it extensible for
later.

If I were to tackle this today, I think the design (which I propose here
for others to review) I would go with would be something like:

- put the fast path of calculating the status into dirstate (or maybe
  its own object)

- figure out if the symlink stuff can also be put into dirstate

- simplify context status code and inheritance

This would allow repo[None].status() to return the dirstate.status (or
some kind of status object) which will be cached and then have
ctx.status(REV) return the on-the-fly calculated status.

This is just a plan off the top of my head, so expect bugs.