Description: ------------ After you introduced my Suggestion in the XML renderer yesterday I found that the XML datasource driver now did not behave consistent, as the exported data could not be loaded back into the same format. So I modified the datasource driver to accept 3 new options: - 'fieldAttribute' to assign the appropriate Attribute to the column field - 'labelAttribute' to assign the appropriate Attribute to the column label - 'dropAttributes' to change the recordset into the same form as if no attributes would be present in the XML data Test script: --------------- Patch submitted; find source at http://www.markusernst.ch/stuff_for_the_world/XML.zip

I am sorry I made a mistake in the patch submitted - the labels array indexes must be the field names instead of numeric indexes. I corrected this in the file provided for download: $labels[] changed to $labels[$item] in lines 117 and 120. Sorry for the inconvenience!

We misunderstood on pear-dev. I thought your link was pointing to the patch. Anyway, I've included your correction and uploaded your patch here : http://phpfi.com/126401 I'm now looking closer...

1 - Please use the .diff extension for your patches, not .txt 2 - Please document the new options in the class DocBlock ("This driver accepts the following options:", etc..) 3 - Use a meaningful variable name instead of $r 4 - Your call to setOptions() override the 'labels' option. Please use array_merge() 5 - I don't really understand the "dropAttributes" option. Do you have an example ?

1 to 4: I apologize for those mistakes - as you asked me to submit a patch rather than posting code in the feature request I tried to understand how to do this now, but this is not too easy without any IT teamwork experience... Do you expect me to do these changes and submit a new patch, or did you mention these points just for my education regarding further contributions? 5: If the XML source contains attributes, the XML Unserializer creates a more complex array; with the dropAttributes option the recordSet is changed to the same form as from other data sources. Dump with dropAttributes false: [recordSet] => Array ( [0] => Array ( [field1] => Array ( [attributes] => Array ( [field] => field1 [label] => E-mail ) [_content] => name@domain.com ) [lang] => Array ( [attributes] => Array ( [field] => lang [label] => User language ) [_content] => Italian ) ) ) The same with the dropAttributes option set to true: [recordSet] => Array ( [0] => Array ( [field1] => name@domain.com [lang] => Italian ) )

Only had a short look so far -- but in line 56: if (isset($info['_content'])) Isn't this too strict? AFAIR it is possible to rename '_content' to another string as the array index in XML_Unserializer.

I'm right, there is an option 'contentName'. http://pear.php.net/manual/en/package.xml.xml-serializer.xml-unserializer.options.php There is no getOption() in XML_Unserializer. We either need to do a feature request there or use something like unset() for the attribute key name (which is one of Markus' options in the XML driver).

You can set the unserializer option: $unserializer->setOption('contentName', '_content'); So there would be no more need for getting the option. (You can also set another name for it of course.)

> So there would be no more need for getting the option. (You can > also set another name for it of course.) Right, what if people use another name? Then your code does not work anymore. (Maybe I just overlook something?)

Markus, if you feel like posting an updated patch for point 1 to 4, that would be great yes.

This will not work with SDG : "If the XML source contains attributes, the XML Unserializer creates a more complex array; with the dropAttributes option the recordSet is changed to the same form as from other data sources" The fetch() method of all datasource drivers must always return an array of the same form. Otherwise SDG will just crash... So the "dropAttributes" option can only be true. Have you tried you XML driver with SDG and "dropAttributes" turned off ? Or are you using this DataSource driver without SDG (as discussed on pear-dev) ?

I made an updated patch with all comments integrated: http://www.markusernst.ch/stuff_for_the_world/Structures_DataGrid.diff Some notes: - setOptions does actually array_merge(); I changed to use setOption anyway. - $unserializer->setOption('contentName', 'whatever') used within the method will set the unserializers option safely, as it overrides this option possibly set anywhere else. - I removed the dropAttributes option - I think the current version will actually crash SDG if the XML source contains attributes? So we found another bug on the fly? I hope my patch is correct and I did not introduce new errors!

I meant : please use array_merge(), in order not to override the labels set by the user, as in : setOptions( 'labels', array_merge($labels, $this->_options['labels'])); There is something that I don't understand, you say : "I removed the dropAttributes option - I think the current version will actually crash SDG if the XML source contains attributes?" When I asked you to remove the "dropAttributes" option, by saying it "can only be true", I meant : it has to be removed, but the driver should behave as if this option is always set to true. That is: always return a proper array, whether there are attributes or not in the XML source. If you have time for changing this, that's great. Thanks for your contribution

Ok the updated patch is online at http://www.markusernst.ch/stuff_for_the_world/Structures_DataGrid.diff I am sorry it contains all changes I made regarding the 3 feature requests I posted (as some of them are in the same files, I could not separate them from each other) - they work quite well in my application and I hope some of it will be considered as useful for the package! Regarding the dropAttributes option I was not expressing myself correctly - what I wanted to say was that I made this option the default behaviour (and thus removed it as an option), and that I assume that the driver version 0.1.0 would break if fed with an XML source that contains attributes.

This patch contains modifications that have strictly no relation with the current request. Please be consistent. Anyway, I have extracted the XML driver diff from this patch, and just applied it. It breaks with my old tests (old style XML, no attributes), while the current driver works fine. With your patch, instead of displaying the fields content, it only displays the first character. That is : | m | 1 | instead of : | myString | 125 | PS: okay, I now understand what you meant about "dropAttributes". Sorry for confusion

Here's my this test script : <?php require_once 'PEAR.php'; PEAR::setErrorHandling(PEAR_ERROR_PRINT); require_once 'Structures/DataGrid.php'; $datagrid =& new Structures_DataGrid(10); $datagrid->bind(file_get_contents('data.xml')); $datagrid->render(DATAGRID_RENDER_CONSOLE); ?> And the XML data : <?xml version="1.0"?> <DataGrid> <Row> <name>Pears</name> <stock>510</stock> <price>1.00</price> </Row> <Row> <name>Apples</name> <stock>210</stock> <price>2.00</price> </Row> </DataGrid> Your patch breaks with these script and data.

Thank you, I will try to fix this, probably next week. I am sorry for the patch - can I just copy the relevant parts out of the whole diff in the future, or should I use different copies of the package for different changes?

You can diff a single file with something like : $ cvs diff DataGrid/DataSource/XML.php You can also manually extract part from a diff file, yes. I wouldn't recommend it though. I'll wait for your updated patch.

I think I fixed it now quite well. Find the patch at http://www.markusernst.ch/stuff_for_the_world/Structures_DataGrid_DataSource_XML.diff It works in my tests with these XML sources with and without attributes: http://www.markusernst.ch/stuff_for_the_world/xml_test_1.xml http://www.markusernst.ch/stuff_for_the_world/xml_test_2.xml http://www.markusernst.ch/stuff_for_the_world/xml_test_3.xml http://www.markusernst.ch/stuff_for_the_world/xml_test_4.xml (The current version only works with the first one.) Test script: <?php require_once 'PEAR.php'; PEAR::setErrorHandling(PEAR_ERROR_PRINT); require_once 'Structures/DataGrid.php'; $options = array( 'generate_columns' => true, 'fieldAttribute' => 'field', 'labelAttribute' => 'label', ); $datagrid =& new Structures_DataGrid(10); $datagrid->bind(file_get_contents('xml_test_1.xml'), $options); $datagrid->render(DATAGRID_RENDER_CONSOLE); ?> It works with "fieldAttribute" and "labelAttribute" options set or not ("labelAttribute" is of no influence if "generate_columns" is false anyway). As for some cases recursive processing is needed, I separated the processing of the rows into a _processRow() method. I hope you like this and I am sorry for having bothered you with my last version which was very buggy indeed.

Great job ! I like _processRow() and the way xml_test_2.xml is handled. You patch has been applied into CVS. This bug is closed. Thank you