[PATCH] dirstate: make subrepos an optional arg to status() and walk()

Augie Fackler durin42 at gmail.com
Mon Feb 15 13:40:19 CST 2010


On Feb 15, 2010, at 1:08 PM, Greg Ward wrote:

> On Mon, Feb 15, 2010 at 11:16 AM, Augie Fackler <durin42 at gmail.com> wrote:
>>>> -        def status(self, match, subrepos, ignored, clean, unknown=True):
>>>> +        def status(self, match, ignored, clean, unknown=True, subrepos=None):
>>> 
>>> FWIW, I originally had it like this, and (if I remember properly) mpm explicitly wanted it moved.
>> 
>> Found it (context: the subrepos arg used to be just "skip" or "skiplist", but mpm deemed
>> that too generic, and also wanted it explicitly clear of the flags in the list):
> 
> Well, I sure never would have found that in the mailing list archive!
> Thanks for digging it up.
> 
>> 2009-12-31, #mercurial:
>> 17:07 < mpm> Let's make skiplist arg after match.
>> 17:07 < durin42> Alright, can do.
>> 17:08 < mpm> And call it subs or subrepos.
>> 17:08 < durin42> subrepos sounds less ambiguous to me, I'll go with that
>> 17:09 < mpm> I think this might be the last iteration before merging.
>> 17:10 < durin42> Cool, starting now, might finish before dinner.
>> 17:10 < durin42> should be quick
>> 17:10 < durin42> skip ok at the end of dirstate.status()?
>> 17:10 < durin42> or right after match there too?
>> 17:10 < durin42> s/skip/subrepos/g
>> 17:10 < mpm> Before the bool args, please.
>> 17:11 < mpm> (flags at the end is a good pattern)
> 
> If this were a virgin greenfield API with no existing code depending
> on it... sure, I'd buy that argument.  But there are third-party
> extensions out there that call dirstate.status() and walk(), and
> others that wrap dirstate.status() and walk().  (I know, because I've
> written one of each.)

I have to ask: why? Outside of something like inotify, I don't think I've ever seen non-core code that hooks into this. Just wondering if there's some more extensible way.

> In general, I'm fine with Mercurial making whatever internal changes
> are necessary, and letting extension authors deal with the
> incompatibilities.  That generally works fine.  

Preaching to the choir - I'm hoping to get some kind of stable Python API eventually, as that'd make out-of-tree extensions much more realistic.

> But in this case, sticking a new arg in the middle of an existing arg list is a real
> pain to deal with from the outside.  In terms of overall pain in the
> world, it's much less pain to stick subrepos at the end of
> dirstate.status() and walk() and make it a keyword arg than to expect
> multiple extensions to implement some vile hack to workaround the
> incompatible change.

I deal with this kind of thing fairly regularly in hgsubversion anyway. It's a fact of life without a stable API (hgsubversion monkeypatches patch.iterhunks(), which is its own "fun" adventure, and hg.parseurl() recently changed, so I'm not really sure I buy your argument about making life easier for out-of-tree extensions much, as this would be a major departure from existing practice).

Also, FYI, mpm is on a cruise for 2 weeks, so we're unlikely to see a change in the status quo until right before the release. Maybe we should make a list of things he should look at pre-release?

> Greg



More information about the Mercurial-devel mailing list