dulwich-users team mailing list archive
-
dulwich-users team
-
Mailing list archive
-
Message #00322
Re: [PATCH 02/28] Add tree-diffing functionality in a 'diff' module.
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?
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 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
>
Attachment:
signature.asc
Description: This is a digitally signed message part
Follow ups
References