[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