dulwich-users team mailing list archive
-
dulwich-users team
-
Mailing list archive
-
Message #00324
Re: [PATCH 02/28] Add tree-diffing functionality in a 'diff' module.
On Mon, Dec 13, 2010 at 04:27, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:
> On Sun, 2010-12-12 at 18:31 -0800, Dave Borowitz wrote:
> > On this topic, did we ever agree on name for this module, like
> > tree_diff instead of diff or something?
> No, I don't think we did. My preference still is to use something other
> than "dulwich.diff". If C git uses diff-tree for this sort of thing then
> I think using something similar ("diff_tree"?) would be reasonable, but
> I have no particular preference. Would diff_tree be ok with you?
>
diff_tree it is.
> Would you like to send a new patchset or should we just do the rename
> after applying the current patchset ? Either works for me.
>
I'll send a new patchset, rebased on your current master.
> I had a look over the rest of your changes this weekend and would be
> happy to merge them once we agree on the name.
>
> Cheers,
>
> Jelmer
>
> > On Sun, Dec 12, 2010 at 15:13, Jelmer Vernooij <jelmer@xxxxxxxxx>
> > wrote:
> > One minor comment on this one:
> >
> >
> > On Thu, 2010-12-02 at 16:21 -0800, dborowitz@xxxxxxxxxx wrote:
> > > index 7c74751..c7f00c0 100644
> > > --- a/dulwich/misc.py
> > > +++ b/dulwich/misc.py
> > > @@ -105,6 +105,7 @@ try:
> > > from collections import namedtuple
> > >
> > > TreeEntryTuple = namedtuple('TreeEntryTuple', ['path',
> > 'mode', 'sha'])
> > > + TreeChangeTuple = namedtuple('TreeChangeTuple',
> > ['type', 'old', 'new'])
> > > except ImportError:
> > > # Provide manual implementations of namedtuples for
> > Python <2.5.
> > > # If the class definitions change, be sure to keep
> > these in sync by running
> > > @@ -153,3 +154,43 @@ except ImportError:
> > > path = _property(_itemgetter(0))
> > > mode = _property(_itemgetter(1))
> > > sha = _property(_itemgetter(2))
> > > +
> > > +
> > > + class TreeChangeTuple(tuple):
> > > + 'TreeChangeTuple(type, old, new)'
> > > +
> > > + __slots__ = ()
> > > +
> > > + _fields = ('type', 'old', 'new')
> > > +
> > > + def __new__(_cls, type, old, new):
> > > + return _tuple.__new__(_cls, (type, old,
> > new))
> > > +
> > > + @classmethod
> > > + def _make(cls, iterable, new=tuple.__new__,
> > len=len):
> > > + 'Make a new TreeChangeTuple object from a
> > sequence or iterable'
> > > + result = new(cls, iterable)
> > > + if len(result) != 3:
> > > + raise TypeError('Expected 3 arguments,
> > got %d' % len(result))
> > > + return result
> > > +
> > > + def __repr__(self):
> > > + return 'TreeChangeTuple(type=%r, old=%r,
> > new=%r)' % self
> > > +
> > > + def _asdict(t):
> > > + 'Return a new dict which maps field names
> > to their values'
> > > + return {'type': t[0], 'old': t[1], 'new':
> > t[2]}
> > > +
> > > + def _replace(_self, **kwds):
> > > + 'Return a new TreeChangeTuple object
> > replacing specified fields with new values'
> > > + result = _self._make(map(kwds.pop, ('type',
> > 'old', 'new'), _self))
> > > + if kwds:
> > > + raise ValueError('Got unexpected field
> > names: %r' % kwds.keys())
> > > + return result
> > > +
> > > + def __getnewargs__(self):
> > > + return tuple(self)
> > > +
> > > + type = _property(_itemgetter(0))
> > > + old = _property(_itemgetter(1))
> > > + new = _property(_itemgetter(2))
> >
> > I'd prefer to keep misc.py limited to compatibility code - on
> > the latest
> > python we should not be importing it at all. We already had
> > TreeEntryTuple in that file so I realise this was already
> > strictly no
> > longer true, and it's hard to do given we don't have
> > namedtuple on older
> > Pythons.
> >
> >
> > Yeah, namedtuples are weird. There is a recipe to just implement
> > namedtuple in pure Python for <2.5, so I guess we could put that in
> > misc.py and then define namedtuples where we need them. I'm not sure
> > about efficiency on 2.4.
> >
> > I'll merge this as is, but it'd be nice if we can come up with
> > a way to
> > avoid it. Perhaps we should also rename misc to _compat or
> > something
> > like that - to discourage users from importing from there.
> >
> >
> > +1 to s/misc/compat/.
> >
> > Cheers,
> >
> > Jelmer
> >
>
>
References