Is there an option to strip whitespace from the XML reports? If not we'll write up a patch to do so if it's not too crazy.

It wouldn't be as simple as creating an option to toggle these values would it?

./renderers/FrxXML.inc: 

 15         $dom->ownerDocument->formatOutput = TRUE;
 16         $dom->ownerDocument->preserveWhiteSpace = TRUE;

Thanks for your guidance

CommentFileSizeAuthor
#6 whitespace_stripping-2238193-6.patch1.02 KBmetzlerd
#3 whitespace_stripping-2238193-3.patch2.23 KBjamesdixon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented

We have large XML files generated by the reports which would be reduced to just over half the size if we were to implement this. :)

metzlerd’s picture

metzlerd CreditAttribution: metzlerd commented

FrxXML is about dispalying xml, and not about .frx file generation. That code is only executed if you use an frx:renderer="FrxXML" attribute in the report. If it cannot be done with simple LibXML configuration objects then you might consider implementing an input filter to do the whitespace replacement and set this as the input filter used by forena. That would reduce the file size, whithout need for complicating the ui.

Another way to look at this would be an skin specified option that was read by the .XML document format. Then it could be applied just before rendering in docformats/FrxXMLDoc.inc. We already have some code there that is checking skin options to control root elements in this case, so it seems like it might be a good fit there.

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Version: 7.x-3.x-dev » 7.x-4.x-dev
FileSize
whitespace_stripping-2238193-3.patch2.23 KB

Rather than stripping it from the output generated inside docformats/FrxXMLDoc.inc, in an effort to reduce memory consumption I've put the whitespace stripping inside renderers/FrxRenderer.inc.

We switched to Forena 4.x, as the memory usage seems to be less in the new version.

Please find the patch attached. To use one can add the following to their skininfo file.

FrxReport[stripWhitespace] = true

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Status: Needs work » Needs review
metzlerd’s picture

metzlerd CreditAttribution: metzlerd commented
Status: Needs review » Needs work

This is the right strategy but I need to refactor this patch based on memory leak work.

metzlerd’s picture

metzlerd CreditAttribution: metzlerd commented
Status: Needs work » Needs review
FileSize
whitespace_stripping-2238193-6.patch1.02 KB

Here's a revised patch that need some testing.

metzlerd’s picture

metzlerd CreditAttribution: metzlerd commented
Status: Needs review » Fixed

Seems to test out as advertised.

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented

I can confirm this works, thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Pierre.Vriens’s picture

Pierre.Vriens CreditAttribution: Pierre.Vriens commented

James or David,

Extra questions related to your "... large XML files generated by the reports which would be reduced to just over half the size ...", and the related .skinfo addition of "FrxReport[stripWhitespace] = true" as described above:

1) Can you think of cases where you would not want to take advantage of this new feature?

2) Assuming that in most (over 50%) of the cases it is best to have this new feature enabled, why not make that the default, either by adding it to the delivered .skinfo file(s), or by swapping the if/else logic (2x) implemented in the current dev version? And in cases where the Whitespaces should not be stripped, add "FrxReport[stripWhitespace] = false".

If this makes sense to you, I'd be happy to submit an appropriate patch for it, either in the .skinfo file, or to swap the logic in the FrxRenderer.inc (let me know which approach is best, but I'd vote for the FrxRenderer.inc correction).

FYI: I'm planning to also document this stripWhitespace option (and some other .skinfo options) in an upcoming iteration of the Tutorial enhancements. So any extra info you can provide on this will help me to do so.

metzlerd’s picture

metzlerd CreditAttribution: metzlerd commented

Forena is not used all that often for generating large XML documents. In this case it was being used to generate an XML document with 10s of thousands of nodes and so the file size was an issue. Generally what you get out of an XML file is the same format as the .frx file that was used to build it and this is the sensible default IMHO. In most cases readability is more important to me than the size of the rendered report, so I would not want it as a default for my reports. Anyone who is concerned about this could make it the default for their custom skin if they chose to. Also there is a small performance penalty (memory and processing) for this feature, so I'm not sure it makes sense to set it as default.