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

Reply via email to