Vincent Siveton wrote:
Hi Dennis,

2007/7/29, Dennis Lundberg <[EMAIL PROTECTED]>:

A couple of thoughts on the changes to xdoc, and in some cases also xhtml:

- From what I can tell the output of EOL chars in XdocSink and XhtmlSink
has changed with this commit. Was that intended? After this commit only
"simple" tags and end tags get an EOL.


Actually intended but need to be improved. See below.


- Changing the tests to ignore EOL chars might be dangerous if we care
about where the EOLs are put.


A lot of modules use EOL or \n inside. We need to review them to be
more consistent and use only system EOL.
About testing, the new AbstractSinkTest class has this convenience
method but like you said, we need to take care about EOL in the
future.

See DOXIA-59.

I think that cases where EOLs are significant should be tested separately as it's usually an exceptional case. It would be quite inconvenient if we had to specify correct EOLs for all the base test methods in AbstractSinkTest. But no principal objection, if someone wants to do it. For now, only the apt module uses the new test classes.

One thing that bothered me in the apt module is actually an issue with the parser, not the sink. It's the reason for point 1 at DOXIA-134: eg the two lines of apt markup

  line 1
  line 2

fire only one sink event when parsed: sink.text( "line 1 line 2" ), I think it should be two separate events, ie sink.text ( "line 1" ); sink.text( "line 2" ), so that EOL's are preserved. WDYT?



- I think we should try to pretty print the sink output to the best of
our capabilities.


Totally agree with you. I think we need to use the
PrettyPrintXMLWriter class from Plexus utils (or other) to correctly
indent the output.I didn't want to change it with this commit, to be
sure to not have side effects. Thoughts?


We have already such an implementation for xdoc: XmlWriterXdocSink. I think it was originally a copy of the XdocSink, just re-written to take an XMLWriter in the constructor, but it hasn't been updated and is not equivalent to XdocSink anymore.


cheers,
-Lukas

Reply via email to