[PATCH 2 of 3 v2] merge: support calculating merge actions against non-working contexts

Matt Mackall mpm at selenic.com
Wed Nov 28 16:47:11 CST 2012


On Tue, 2012-11-27 at 17:13 -0800, Bryan O'Sullivan wrote:
> On Tue, Nov 27, 2012 at 3:36 PM, David Schleimer <dschleimer at fb.com> wrote:
> 
> >          # collision check is not needed for clean update
> >          if (not branchmerge and
> > -            (force or not tctx.dirty(missing=True, branch=False))):
> > +            (force or (isinstance(tctx, context.workingctx) and
> > +                       not tctx.dirty(missing=True, branch=False)))):
> >              _checkcollision(mctx, None)
> >          else:
> >              _checkcollision(mctx, tctx)
> >      if not force:
> >          _checkunknown(repo, tctx, mctx)
> > -    action += _forgetremoved(tctx, mctx, branchmerge)
> > +    if isinstance(tctx, context.workingctx):
> > +        action += _forgetremoved(tctx, mctx, branchmerge)
> >
> 
> These uses of isinstance make me wince.

Yeah, I noticed that too. Having a dirty() method on the base type would
be better.

> The first one could be addressed by adding a no-op dirty method to whatever
> type tctx is when it's not a workingctx, and have it always return the
> right value for the branch to evaluate correctly. That would make the code
> easier to understand, I think.
> 
> Maybe something similar could be done for the second case, too? I haven't
> thought about that in any depth.

Elsewhere, we use "if ctx.rev() == None" to distinguish historic from
non-historic contexts. But that might not be the distinction we want
here.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list