[PATCH 6 of 8] dirstate: create class for status lists

Martin von Zweigbergk martinvonz at gmail.com
Thu Oct 2 08:12:16 CDT 2014


On Thu, Oct 2, 2014 at 5:07 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> On Wed, 1 Oct 2014 22:00:15 -0700, Martin von Zweigbergk wrote:
>> >>   +class status(object):
>> >
>> >
>> > Please describe the intention with this class.
>> >
>> >> +    def __init__(self, modified, added, removed, deleted, unknown,
>> >> +                ignored, clean):
>> >> +        self.modified = modified
>> >> +        self.added = added
>> >> +        self.removed = removed
>> >> +        self.deleted = deleted
>> >> +        self.unknown = unknown
>> >> +        self.ignored = ignored
>> >> +        self.clean = clean
>> >
>> >
>> > http://mercurial.selenic.com/wiki/CodingStyle#Classes ... but I agree it is
>> > nicer than the 7-tuples.
>>
>> Heh, I guess I should have read that first. I guess "recovering Java
>> programmer" describes me well, but I'm not sure I thought three times.
>> I did, however, ask on the IRC channel and heard no complaints (and no
>> encouragement either). I know Augie is for it.
>>
>> > It is also very similar to a named tuple. Could we use that instead ... or
>> > inherit from it? (That would require some kludge as long as we support
>> > Python < 2.6.)
>>
>> After having read the style guide, I know that extending from it is
>> not an option :-) I'll let others discuss and decide, since I know
>> nothing about Python. The ~8 lines of constructor doesn't seem that
>> bad to me.
>
> It's fairly easy to build namedtuple-like object.
>
> https://hg.python.org/cpython/file/v2.7.8/Lib/collections.py#l234
>
>     class statustuple(tuple):
>         __slots__ = ()
>
>         def __new__(cls, modified, added, ...):
>             return tuple.__new__(cls, (modified, added, ...))
>
>         @property
>         def modified(self):
>             return self[0]
>         ...
>
>         def __repr__(self):
>             ...
>
> It can avoid breaking third-party extensions (and thg) that expects status()
> returns a tuple.
>
> Regards,

Sure, I'm happy to make it extend tuple if others agree. I actually
considered letting the class extend tuple temporarily in the patch
series to make the diffs smaller, but I don't really want it to be a
tuple in the end (I'd prefer if new instances of "status[4]" could not
be added later).

I'll also let the rest of you reach decide whether to use namedtuple.
After reading about them for about a minute, I can't answer that
myself.


More information about the Mercurial-devel mailing list