Hi Florent!
Florent Guillaume wrote:
On 3 Apr 2005, at 17:27, yuppie wrote:Florent Guillaume wrote:
- all in all it's quite nice,
Well. It works for some use cases, but there is yet a lot to be done. Though its maturity is still alpha, it already contains a lot of cruft.
Yes, I think I'll be doing some cleanups in it if you don't object.
Go ahead!
And probably apply http://www.artima.com/weblogs/viewpost.jsp?thread=101968 to the code in general... I really have a problem with Tres's style ;)
Discuss that with Tres ;)
- writing the code for the import handler is really painful, I wish we could use an API like elementtree. Couldn't we include it ?
Could you please elaborate on this? What are the most painful parts? Why are they painful?
Finding how to write the import mapping.
I have to admit it is not well documented.
For instance I have this stucture:
<?xml version="1.0"?>
<directory kind="CPS Members Directory">
<properties>
<property name="title">label_members</property>
<property name="schema">members</property>
[... more properties ...]
</properties>
<entry-local-roles>
<entry-local-role role="Owner">python:entry_id == user_id</entry-local-role>
</entry-local-roles>
</directory>
I would use this structure instead:
<?xml version="1.0"?>
<directory kind="CPS Members Directory">
<property name="title">label_members</property>
<property name="schema">members</property>
[... more properties ...]
<entry-local-role role="Owner">python:entry_id == user_id</entry-local-role>
</directory>
But I'll explain it for your example.
Which I parse using:
def _getImportMapping(self): return { 'directory': { 'kind': {}, 'properties': {KEY: 'props'}, 'entry-local-roles': {KEY: 'elrs', DEFAULT: ()},
To get rid of the intermediate [0], use
'properties': {KEY: 'props', CONVERTER: self._convertToUnique}, 'entry-local-roles': {KEY: 'elrs', DEFAULT: ((),), CONVERTER: self._convertToUnique},
Currently the converter is called after setting the default. I guess it would be better not to call the converter in that case.
}, 'properties': { 'property': {KEY: 'properties'},
To get rid of the intermediate ['properties'], use
'property': {KEY: None},
}, 'entry-local-roles': { 'entry-local-role': {KEY: 'elr'},
To get rid of the intermediate ['elr'], use
'entry-local-role': {KEY: None},
}, 'entry-local-role': { 'role': {}, '#text': {KEY: 'expr'}, }, }
And I have to do:
dir_info = dconf.parseXML(dir_text) dir = tool.manage_addCPSDirectory(dir_id, dir_info['kind']) for prop_info in dir_info['props'][0]['properties']: dconf.initProperty(dir, prop_info) dir._postProcessProperties() for elr in dir_info['elrs']: elr = elr['elr'][0]
Did this work? I would have expected a line like this:
for elr in dir_info['elrs'][0]['elr']:
And with the changes proposed above this should work:
for elr in dir_info['elrs']:
res = dir.addEntryLocalRole(elr['role'], elr['expr'])
All the intermediate [0] and ['elr'] are cruft I'd like to get rid of. Maybe it's because I don't know how to use it best...
My comments are untested, but I hope they are helpful anyway. See rolemap.py for a similar example.
Are you talking about the import handlers themselves, the configurator classes or also about export handlers?
I have a question about a pattern used in the configurator classes: many times I saw the pattern than only one configurator was instanciated, for instance TypeInfoConfigurator, and then in the loop the ZPT called by passing, for instance:
tic.generateXML( type_id=type_id )
and the ZPT itself passes back type_id to getTypeInfo. Why not simply have the configurator instanciated once per type, with the type itself as an attribute ? Is it just old cruft ?
Don't have strong feelings about that. But the change you propose seems to make it easier to implement the next step: Using different configurators for different TypeInfo classes.
Cheers,
Yuppie
_______________________________________________ Zope-CMF maillist - Zope-CMF@lists.zope.org http://mail.zope.org/mailman/listinfo/zope-cmf
See http://collector.zope.org/CMF for bug reports and feature requests