Re: svn commit: r560737 [1/2] - in /maven/doxia/doxia/trunk/doxia-modules: doxia-module-xdoc/src/main/java/org/apache/maven/doxia/module/xdoc/ doxia-module-xdoc/src/test/java/org/apache/maven/doxia/module/xdoc/ doxia-module-xhtml/src/main/java/org/ap

[Vincent Siveton <vi...@gmail.com>]

Hi Dennis,

2007/7/29, Dennis Lundberg <de...@apache.org>:
> 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.

> - 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?

Thanks,

Vincent

>
> vsiveton@apache.org wrote:
> > Author: vsiveton
> > Date: Sun Jul 29 08:40:13 2007
> > New Revision: 560737
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=560737
> > Log:
> > o Put all hardcoded markups in an interface
> > o using javax.swing.text.html.HTML.Tag and javax.swing.text.html.HTML.Attribute instead of harcoded tags/attributes
> > o updated javadocs
> >

[SNIP]

Re: svn commit: r560737 [1/2] - in /maven/doxia/doxia/trunk/doxia-modules: doxia-module-xdoc/src/main/java/org/apache/maven/doxia/module/xdoc/ doxia-module-xdoc/src/test/java/org/apache/maven/doxia/module/xdoc/ doxia-module-xhtml/src/main/java/org/ap

[Lukas Theussl <lt...@apache.org>]

Vincent Siveton wrote:
> Hi Dennis,
> 
> 2007/7/29, Dennis Lundberg <de...@apache.org>:
> 
>>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