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

Matt Mackall mpm at selenic.com
Thu Oct 2 12:58:19 CDT 2014


On Thu, 2014-10-02 at 06:12 -0700, Martin von Zweigbergk wrote:
> 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).

A named tuple with slots is the way to go. It also leads to a much
smaller initial patch. I actually wouldn't go to the trouble of changing
all the users right away - a lot of them are implicitly assigning things
to local vars as a micro-optimization.

(The secret to working with the current tuple, by the way is to remember
the string 'marduic' which match the option flags of status and is also
the name of the Babylonian god of status results.)

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list