[MERGE] Use slots consistently in InventoryEntry hierarchy -- regenerated against new head. Seeking another reviewer.

[Jan Hudec]

On Wed, May 31, 2006 at 09:15:28 -0400, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Jan Hudec wrote:
> > On Tue, May 30, 2006 at 19:23:44 -0400, Aaron Bentley wrote:
> >>a) Each InventoryEntry could provide a get_attribute_list() method,
> >>which would provide a list of name,value tuples.
> >>
> >>b) Each InventoryEntry could provide a set_attributes() method, which
> >>would take as an argument an object that it can store its attributes in
> > 
> > 
> > These two are a good idea. Thanks.
> > 
> > I want to do the properties stuff. I originally wanted to convert the
> > base attributes to slots (with the patch I sent) and add a proplist
> > dictionary with the other ones (executable + properties defined by
> > plugins (eg. newline-style, keyword expansion, charset conversion, unix
> > permissions etc.). Now that would add a dictionary back, right? 
> 
> It seems almost certain.
> 
> The reason I mentioned a list of tuples rather than a dict is because I
> think it would be a good idea for attributes to have a defined order.
> 
> But maybe you just have to provide an iterator that provides results in
> sorted order.
> 
> > And
> > quite many operations would have to look at it anyway. So this leads me
> > to thinking that perhaps I should unify the core and non-core attributes
> > and use methods like you suggested for everything, not just the
> > proplist.
> 
> This sounds like it would be slower than using slots, and possibly
> slower than normal variable access, so please benchmark if you make that
> kind of change.

Yes. I will try to benchmark and optimize before posting any patch.

With proplist the access would indeed be slower. That's why I am
thinking about keeping slots for the core properties (they are still
accessed more often and this will speed up the access) and __dict__ for
the non-core ones.

> Also, it sounds like there should be namespaces for properties, so we
> don't get people accidentally overriding core bzr attributes.

You are right. Well if I use __dict__ for them (and it should make them
slightly faster), they could conflict even with methods of
InventoryEntry and I don't like renaming those. So I'd say perhaps core
properties would use simple names and plugin ones would be required to
take form of <pluginname>__<propertname> (double underscore, since plain
name can contain underscore as well (eg. text_sha1)). The
bzrlib.properties.register_property function can ensure that -- core
properties will be defined in the bzrlib.properties module itself, so
they can be inserted in the registry directly.

-- 
						 Jan 'Bulb' Hudec <bulb at ucw.cz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060531/650682ce/attachment.pgp