[Zope-CMF] Re: CMFSetup comments

[yuppie]

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