https://issues.apache.org/bugzilla/show_bug.cgi?id=47311
--- Comment #4 from Vincent Hennebert <vhenneb...@gmail.com> 2009-06-26 03:08:28 PST --- I've had a more detailed look at the patch and it looks good to me. A few other comments: - the definition and parsing of the extension properties should not be put in ExtensionElementMapping, but in a class (and a sub-package) of its own. Where exactly it should be put is not entirely clear to me yet. Probably in a new o.a.f.render.extensions package. How to name the new sub-package also is an open question (scalingcropbleedtrim? rather ugly... pageboundaries (taken from the PDF spec)? prepress?). - amendment to what I said about the event notification mechanism: the extension should not use it, rather throw appropriate exceptions, which would be passed over by the PDF library (the classes in o.a.f.pdf) to the PDFDocumentHandler. The former two should remain independent of the notification mechanism to allow later extraction from the FOP codebase and modularization. Only the PDFDocumentHandler must be aware of the notification mechanism. - the regexp parsing properties should be made slightly more robust and unit-tested. - I have a concern about when the properties are actually parsed. Like it is now they will be parsed only at rendering stage, so if there is a mistake in them the error will be thrown rather 'late' in the process (only after layout has been performed). May that be a problem? Do we want a 'fail-fast' behaviour instead? Open question. I'm happy to do all the mentioned changes myself, but this will be in one week at the earliest as I'm away next week. Peter, if meanwhile you want to submit an updated patch feel free to do so :-) Thanks, Vincent -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
