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

David Schleimer dschleimer at fb.com
Sat Dec 1 00:38:51 UTC 2012


> > These uses of isinstance make me wince.
> 
> Yeah, I noticed that too. Having a dirty() method on the base type would be better.

I did it this way because memctx doesn't extend changectx, and when I wrote this I wasn't clear on exactly what made sense to pass here.  Now that I have a better understanding of what memctx is for, I agree it doesn't make any sense to pass a memctx.  I'll update changectx to have a return-false dirty method.

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

deleted() and removed() seem like less-good things to put on changectx.  I would (perhaps naively) expect them to return files deleted by the changeset the changectx represents by analogy with the file_dels template.  That's not what we would want them to do here, however.  We would instead want them to return an empty list immediately.  I think this would be sufficiently confusing that it would be better to use "ctx.rev() is None" to detect non-historic contexts and decide whether to run _forgetremoved()

--David


More information about the Mercurial-devel mailing list