Re: svn commit: r598803 - in /maven/doxia/doxia/trunk: doxia-core/src/main/java/org/apache/maven/doxia/sink/ doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/ doxia-modules/doxia-module-fo/src/main/java/org/apa

[Lukas Theussl-3]

Herve,

Thanks for fixing this! Just two questions:

Why do we need a separate method in AbstractXmlSink, can't we just
remove the EOL from writeEndTag()?

And what's the reason for selecting only special tags to write no
newline? Just because they are inline elements? This doesn't solve the
issue of EOLs within <pre> tags, right?

Cheers,
-Lukas


hboutemy@... wrote:

> Author: hboutemy
> Date: Tue Nov 27 15:02:47 2007
> New Revision: 598803
>
> URL: http://svn.apache.org/viewvc?rev=598803&view=rev
> Log:
> [DOXIA-189] removed newline added after anchor, link, bold, italic and monospaced tags
>
> Modified:
>     maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/AbstractXmlSink.java
>     maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/XhtmlBaseSink.java
>     maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/DocBookSink.java
>     maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java
>     maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTest.java
>
> Modified: maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/AbstractXmlSink.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/AbstractXmlSink.java?rev=598803&r1=598802&r2=598803&view=diff
> ==============================================================================
> --- maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/AbstractXmlSink.java (original)
> +++ maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/AbstractXmlSink.java Tue Nov 27 15:02:47 2007
> @@ -166,9 +166,8 @@
>      }
>  
>      /**
> -     * Ends a Tag. For instance:
> -     * <pre>
> -     * </tag>
> +     * Ends a Tag followed by an EOL. For instance:
> +     * <pre></tag>
>       * </pre>
>       *
>       * @param t a tag
> @@ -188,6 +187,29 @@
>          sb.append( String.valueOf( GREATER_THAN ) );
>  
>          sb.append( EOL );
> +
> +        write( sb.toString() );
> +    }
> +
> +    /**
> +     * Ends a Tag without an EOL. For instance:
> +     * <pre></tag></pre>
> +     *
> +     * @param t a tag
> +     */
> +    protected void writeEndTagWithoutEOL( Tag t )
> +    {
> +        StringBuffer sb = new StringBuffer();
> +        sb.append( String.valueOf( LESS_THAN ) );
> +        sb.append( String.valueOf( SLASH ) );
> +
> +        if ( nameSpace != null )
> +        {
> +            sb.append( nameSpace ).append( ":" );
> +        }
> +
> +        sb.append( t.toString() );
> +        sb.append( String.valueOf( GREATER_THAN ) );
>  
>          write( sb.toString() );
>      }
>
> Modified: maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/XhtmlBaseSink.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/XhtmlBaseSink.java?rev=598803&r1=598802&r2=598803&view=diff
> ==============================================================================
> --- maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/XhtmlBaseSink.java (original)
> +++ maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/sink/XhtmlBaseSink.java Tue Nov 27 15:02:47 2007
> @@ -907,7 +907,7 @@
>      {
>          if ( !headFlag )
>          {
> -            writeEndTag( Tag.A );
> +            writeEndTagWithoutEOL( Tag.A );
>          }
>      }
>  
> @@ -990,7 +990,7 @@
>      {
>          if ( !headFlag )
>          {
> -            writeEndTag( Tag.A );
> +            writeEndTagWithoutEOL( Tag.A );
>          }
>      }
>  
> @@ -1014,7 +1014,7 @@
>      {
>          if ( !headFlag )
>          {
> -            writeEndTag( Tag.I );
> +            writeEndTagWithoutEOL( Tag.I );
>          }
>      }
>  
> @@ -1038,7 +1038,7 @@
>      {
>          if ( !headFlag )
>          {
> -            writeEndTag( Tag.B );
> +            writeEndTagWithoutEOL( Tag.B );
>          }
>      }
>  
> @@ -1062,7 +1062,7 @@
>      {
>          if ( !headFlag )
>          {
> -            writeEndTag( Tag.TT );
> +            writeEndTagWithoutEOL( Tag.TT );
>          }
>      }
>  
>
> Modified: maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/DocBookSink.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/DocBookSink.java?rev=598803&r1=598802&r2=598803&view=diff
> ==============================================================================
> --- maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/DocBookSink.java (original)
> +++ maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/DocBookSink.java Tue Nov 27 15:02:47 2007
> @@ -1478,7 +1478,7 @@
>          {
>              if ( !xmlMode )
>              {
> -                writeEndTag( ANCHOR_TAG );
> +                writeEndTagWithoutEOL( ANCHOR_TAG );
>              }
>          }
>      }
> @@ -1521,12 +1521,12 @@
>      {
>          if ( externalLinkFlag )
>          {
> -            writeEndTag( ULINK_TAG );
> +            writeEndTagWithoutEOL( ULINK_TAG );
>              externalLinkFlag = false;
>          }
>          else
>          {
> -            writeEndTag( LINK_TAG );
> +            writeEndTagWithoutEOL( LINK_TAG );
>          }
>      }
>  
>
> Modified: maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java?rev=598803&r1=598802&r2=598803&view=diff
> ==============================================================================
> --- maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java (original)
> +++ maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java Tue Nov 27 15:02:47 2007
> @@ -780,7 +780,7 @@
>      /** {@inheritDoc} */
>      public void anchor_()
>      {
> -        writeEndTag( INLINE_TAG );
> +        writeEndTagWithoutEOL( INLINE_TAG );
>      }
>  
>      /** {@inheritDoc} */
> @@ -815,8 +815,8 @@
>      /** {@inheritDoc} */
>      public void link_()
>      {
> -        writeEndTag( INLINE_TAG );
> -        writeEndTag( BASIC_LINK_TAG );
> +        writeEndTagWithoutEOL( INLINE_TAG );
> +        writeEndTagWithoutEOL( BASIC_LINK_TAG );
>      }
>  
>      /** {@inheritDoc} */
> @@ -828,7 +828,7 @@
>      /** {@inheritDoc} */
>      public void italic_()
>      {
> -        writeEndTag( INLINE_TAG );
> +        writeEndTagWithoutEOL( INLINE_TAG );
>      }
>  
>      /** {@inheritDoc} */
> @@ -840,7 +840,7 @@
>      /** {@inheritDoc} */
>      public void bold_()
>      {
> -        writeEndTag( INLINE_TAG );
> +        writeEndTagWithoutEOL( INLINE_TAG );
>      }
>  
>      /** {@inheritDoc} */
> @@ -852,7 +852,7 @@
>      /** {@inheritDoc} */
>      public void monospaced_()
>      {
> -        writeEndTag( INLINE_TAG );
> +        writeEndTagWithoutEOL( INLINE_TAG );
>      }
>  
>      /** {@inheritDoc} */
>
> Modified: maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTest.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTest.java?rev=598803&r1=598802&r2=598803&view=diff
> ==============================================================================
> --- maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTest.java (original)
> +++ maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTest.java Tue Nov 27 15:02:47 2007
> @@ -142,7 +142,7 @@
>      protected String getTableBlock( String cell, String caption )
>      {
>          // TODO: something's wrong
> -        return "\\begin{ptable}\\begin{ptablerows}{c}\\begin{pcell}{c}cell\\end{pcell}\\\\\\end{ptablerows}\\ptablecaption{Table caption}\\end{ptable}";
> +        return "\\begin{ptable}\\begin{ptablerows}{c}\\begin{pcell}{c}cell\\end{pcell}\\\\\\end{ptablerows}\\ptablecaption{" + caption + "}\\end{ptable}";
>      }
>  
>      /** {@inheritDoc} */
>
>
>

[Hervé BOUTEMY]

Le mercredi 28 novembre 2007, Lukas Theussl a écrit :
> Herve,
>
> Thanks for fixing this! Just two questions:
>
> Why do we need a separate method in AbstractXmlSink, can't we just
> remove the EOL from writeEndTag()?
I tried to do so in the first place: that's exactly what I wrote when mailing
to maven-dev.
But when doing so, I discovered 2 drawbacks:
- the whole generated HTML document was without any EOL, which is quite hard
to read. But no major problem here, I just thought that people who had
written these newlines prefer readable HTML, and wouldn't be happy if I
changed their "way of code" :)
- unit test were failing, since some sinks use LineBreaker class which adds a
newline when larger than 78 chars: the whole unit tests logic had to be
reworked...
Then I discovered where really these newlines were a problem, and where they
were not: see next paragraph...

>
> And what's the reason for selecting only special tags to write no
> newline? Just because they are inline elements? This doesn't solve the
> issue of EOLs within <pre> tags, right?
Yes, because they're inline elements: HTML <a>, <i>, <b> and <code>. Within
<pre> tag, you don't use other tags, which are structural tags: you only use
inline elements for styling and links/anchors.
FYI, here is a page on which I discovered the problem when using 2.0-beta6
http://logdistiller.sourceforge.net/quickstart-4.html

Feel free to check and comment this logic, because I think it is good but I'm
not 100% sure I didn't miss a special case...

regards

Hervé

>
> Cheers,
> -Lukas
>
> hboutemy@... wrote:
> > Author: hboutemy
> > Date: Tue Nov 27 15:02:47 2007
> > New Revision: 598803
> >
> > URL: http://svn.apache.org/viewvc?rev=598803&view=rev
> > Log:
> > [DOXIA-189] removed newline added after anchor, link, bold, italic and
> > monospaced tags
> >
> > Modified:
> >    
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/AbstractXmlSink.java
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/XhtmlBaseSink.java
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >n/java/org/apache/maven/doxia/module/docbook/DocBookSink.java
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/a
> >pache/maven/doxia/module/fo/FoSink.java
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/or
> >g/apache/maven/doxia/module/latex/LatexSinkTest.java
> >
> > Modified:
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/AbstractXmlSink.java URL:
> > http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >java/org/apache/maven/doxia/sink/AbstractXmlSink.java?rev=598803&r1=598802
> >&r2=598803&view=diff
> > =========================================================================
> >===== ---
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/AbstractXmlSink.java (original) +++
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/AbstractXmlSink.java Tue Nov 27 15:02:47 2007 @@ -166,9 +166,8 @@
> >      }
> >
> >      /**
> > -     * Ends a Tag. For instance:
> > -     * <pre>
> > -     * </tag>
> > +     * Ends a Tag followed by an EOL. For instance:
> > +     * <pre></tag>
> >       * </pre>
> >       *
> >       * @param t a tag
> > @@ -188,6 +187,29 @@
> >          sb.append( String.valueOf( GREATER_THAN ) );
> >
> >          sb.append( EOL );
> > +
> > +        write( sb.toString() );
> > +    }
> > +
> > +    /**
> > +     * Ends a Tag without an EOL. For instance:
> > +     * <pre></tag></pre>
> > +     *
> > +     * @param t a tag
> > +     */
> > +    protected void writeEndTagWithoutEOL( Tag t )
> > +    {
> > +        StringBuffer sb = new StringBuffer();
> > +        sb.append( String.valueOf( LESS_THAN ) );
> > +        sb.append( String.valueOf( SLASH ) );
> > +
> > +        if ( nameSpace != null )
> > +        {
> > +            sb.append( nameSpace ).append( ":" );
> > +        }
> > +
> > +        sb.append( t.toString() );
> > +        sb.append( String.valueOf( GREATER_THAN ) );
> >
> >          write( sb.toString() );
> >      }
> >
> > Modified:
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/XhtmlBaseSink.java URL:
> > http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-core/src/main/
> >java/org/apache/maven/doxia/sink/XhtmlBaseSink.java?rev=598803&r1=598802&r
> >2=598803&view=diff
> > =========================================================================
> >===== ---
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/XhtmlBaseSink.java (original) +++
> > maven/doxia/doxia/trunk/doxia-core/src/main/java/org/apache/maven/doxia/s
> >ink/XhtmlBaseSink.java Tue Nov 27 15:02:47 2007 @@ -907,7 +907,7 @@
> >      {
> >          if ( !headFlag )
> >          {
> > -            writeEndTag( Tag.A );
> > +            writeEndTagWithoutEOL( Tag.A );
> >          }
> >      }
> >
> > @@ -990,7 +990,7 @@
> >      {
> >          if ( !headFlag )
> >          {
> > -            writeEndTag( Tag.A );
> > +            writeEndTagWithoutEOL( Tag.A );
> >          }
> >      }
> >
> > @@ -1014,7 +1014,7 @@
> >      {
> >          if ( !headFlag )
> >          {
> > -            writeEndTag( Tag.I );
> > +            writeEndTagWithoutEOL( Tag.I );
> >          }
> >      }
> >
> > @@ -1038,7 +1038,7 @@
> >      {
> >          if ( !headFlag )
> >          {
> > -            writeEndTag( Tag.B );
> > +            writeEndTagWithoutEOL( Tag.B );
> >          }
> >      }
> >
> > @@ -1062,7 +1062,7 @@
> >      {
> >          if ( !headFlag )
> >          {
> > -            writeEndTag( Tag.TT );
> > +            writeEndTagWithoutEOL( Tag.TT );
> >          }
> >      }
> >
> >
> > Modified:
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >n/java/org/apache/maven/doxia/module/docbook/DocBookSink.java URL:
> > http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-
> >module-docbook-simple/src/main/java/org/apache/maven/doxia/module/docbook/
> >DocBookSink.java?rev=598803&r1=598802&r2=598803&view=diff
> > =========================================================================
> >===== ---
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >n/java/org/apache/maven/doxia/module/docbook/DocBookSink.java (original)
> > +++
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-docbook-simple/src/mai
> >n/java/org/apache/maven/doxia/module/docbook/DocBookSink.java Tue Nov 27
> > 15:02:47 2007 @@ -1478,7 +1478,7 @@
> >          {
> >              if ( !xmlMode )
> >              {
> > -                writeEndTag( ANCHOR_TAG );
> > +                writeEndTagWithoutEOL( ANCHOR_TAG );
> >              }
> >          }
> >      }
> > @@ -1521,12 +1521,12 @@
> >      {
> >          if ( externalLinkFlag )
> >          {
> > -            writeEndTag( ULINK_TAG );
> > +            writeEndTagWithoutEOL( ULINK_TAG );
> >              externalLinkFlag = false;
> >          }
> >          else
> >          {
> > -            writeEndTag( LINK_TAG );
> > +            writeEndTagWithoutEOL( LINK_TAG );
> >          }
> >      }
> >
> >
> > Modified:
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/a
> >pache/maven/doxia/module/fo/FoSink.java URL:
> > http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-
> >module-fo/src/main/java/org/apache/maven/doxia/module/fo/FoSink.java?rev=5
> >98803&r1=598802&r2=598803&view=diff
> > =========================================================================
> >===== ---
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/a
> >pache/maven/doxia/module/fo/FoSink.java (original) +++
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-fo/src/main/java/org/a
> >pache/maven/doxia/module/fo/FoSink.java Tue Nov 27 15:02:47 2007 @@ -780,7
> > +780,7 @@
> >      /** {@inheritDoc} */
> >      public void anchor_()
> >      {
> > -        writeEndTag( INLINE_TAG );
> > +        writeEndTagWithoutEOL( INLINE_TAG );
> >      }
> >
> >      /** {@inheritDoc} */
> > @@ -815,8 +815,8 @@
> >      /** {@inheritDoc} */
> >      public void link_()
> >      {
> > -        writeEndTag( INLINE_TAG );
> > -        writeEndTag( BASIC_LINK_TAG );
> > +        writeEndTagWithoutEOL( INLINE_TAG );
> > +        writeEndTagWithoutEOL( BASIC_LINK_TAG );
> >      }
> >
> >      /** {@inheritDoc} */
> > @@ -828,7 +828,7 @@
> >      /** {@inheritDoc} */
> >      public void italic_()
> >      {
> > -        writeEndTag( INLINE_TAG );
> > +        writeEndTagWithoutEOL( INLINE_TAG );
> >      }
> >
> >      /** {@inheritDoc} */
> > @@ -840,7 +840,7 @@
> >      /** {@inheritDoc} */
> >      public void bold_()
> >      {
> > -        writeEndTag( INLINE_TAG );
> > +        writeEndTagWithoutEOL( INLINE_TAG );
> >      }
> >
> >      /** {@inheritDoc} */
> > @@ -852,7 +852,7 @@
> >      /** {@inheritDoc} */
> >      public void monospaced_()
> >      {
> > -        writeEndTag( INLINE_TAG );
> > +        writeEndTagWithoutEOL( INLINE_TAG );
> >      }
> >
> >      /** {@inheritDoc} */
> >
> > Modified:
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/or
> >g/apache/maven/doxia/module/latex/LatexSinkTest.java URL:
> > http://svn.apache.org/viewvc/maven/doxia/doxia/trunk/doxia-modules/doxia-
> >module-latex/src/test/java/org/apache/maven/doxia/module/latex/LatexSinkTe
> >st.java?rev=598803&r1=598802&r2=598803&view=diff
> > =========================================================================
> >===== ---
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/or
> >g/apache/maven/doxia/module/latex/LatexSinkTest.java (original) +++
> > maven/doxia/doxia/trunk/doxia-modules/doxia-module-latex/src/test/java/or
> >g/apache/maven/doxia/module/latex/LatexSinkTest.java Tue Nov 27 15:02:47
> > 2007 @@ -142,7 +142,7 @@
> >      protected String getTableBlock( String cell, String caption )
> >      {
> >          // TODO: something's wrong
> > -        return
> > "\\begin{ptable}\\begin{ptablerows}{c}\\begin{pcell}{c}cell\\end{pcell}\\
> >\\\\end{ptablerows}\\ptablecaption{Table caption}\\end{ptable}"; +      
> > return
> > "\\begin{ptable}\\begin{ptablerows}{c}\\begin{pcell}{c}cell\\end{pcell}\\
> >\\\\end{ptablerows}\\ptablecaption{" + caption + "}\\end{ptable}"; }
> >
> >      /** {@inheritDoc} */

[Lukas Theussl-3]

Hi,

We had a discussion about EOLs some time ago, I would like to know which
tests failed for you. The AbstractSinkTest (which all sink tests are
supposed to extend) filters out every EOL in the strings it compares (my
reasoning for that was that the case where EOLs are significant is
usually a special case and should be tested separately).

Apart from that, IMO AbstractXmlSink is an abstract base class that
shouldn't be concerned with formatting (btw, I just noticed that
writeStartTag also appends an EOL after simple tags, that should be
removed as well). The writeEndTagWithoutEOL() method is simply not
needed because implementing sinks should write the EOLs where they need
them. In the current case, it should be enough to move the EOLs into the
XhtmlBaseSink.

If tests fail after that then it's probaby the tests that need to be
adapted...

Cheers,
-Lukas

PS Are you saying you are using site-plugin beta-6 with doxia beta-1? I
haven't been able to make the two work together yet...



Hervé BOUTEMY wrote:

> Le mercredi 28 novembre 2007, Lukas Theussl a écrit :
>
>>Herve,
>>
>>Thanks for fixing this! Just two questions:
>>
>>Why do we need a separate method in AbstractXmlSink, can't we just
>>remove the EOL from writeEndTag()?
>
> I tried to do so in the first place: that's exactly what I wrote when mailing
> to maven-dev.
> But when doing so, I discovered 2 drawbacks:
> - the whole generated HTML document was without any EOL, which is quite hard
> to read. But no major problem here, I just thought that people who had
> written these newlines prefer readable HTML, and wouldn't be happy if I
> changed their "way of code" :)
> - unit test were failing, since some sinks use LineBreaker class which adds a
> newline when larger than 78 chars: the whole unit tests logic had to be
> reworked...
> Then I discovered where really these newlines were a problem, and where they
> were not: see next paragraph...
>
>
>>And what's the reason for selecting only special tags to write no
>>newline? Just because they are inline elements? This doesn't solve the
>>issue of EOLs within <pre> tags, right?
>
> Yes, because they're inline elements: HTML <a>, <i>, <b> and <code>. Within
> <pre> tag, you don't use other tags, which are structural tags: you only use
> inline elements for styling and links/anchors.
> FYI, here is a page on which I discovered the problem when using 2.0-beta6
> http://logdistiller.sourceforge.net/quickstart-4.html
>
> Feel free to check and comment this logic, because I think it is good but I'm
> not 100% sure I didn't miss a special case...
>
> regards
>
> Hervé
>
>>Cheers,
>>-Lukas
>>

[Hervé BOUTEMY]

Le jeudi 29 novembre 2007, Lukas Theussl a écrit :
> Hi,
>
> We had a discussion about EOLs some time ago, I would like to know which
> tests failed for you. The AbstractSinkTest (which all sink tests are
> supposed to extend) filters out every EOL in the strings it compares (my
> reasoning for that was that the case where EOLs are significant is
> usually a special case and should be tested separately).
Latex tests failed. The reason was that there was text with spaces in it
("Numbered list", "Item list", ...), and when the LineBreaker broke a long
line on this space, removing the EOL led to "Numberedlist".

>
> Apart from that, IMO AbstractXmlSink is an abstract base class that
> shouldn't be concerned with formatting (btw, I just noticed that
> writeStartTag also appends an EOL after simple tags, that should be
> removed as well). The writeEndTagWithoutEOL() method is simply not
> needed because implementing sinks should write the EOLs where they need
> them. In the current case, it should be enough to move the EOLs into the
> XhtmlBaseSink.
why not. I didn't mean to change everything, just the simplest way to get
things working. I think both approaches are right: between
writeEndTagWithoutEOL() and writeEndTag(), every Sink chooses which method is
right for on each call.

>
> If tests fail after that then it's probaby the tests that need to be
> adapted...
Yes, with LineBreaker in place, the test strategy is not perfect: it fails
when tests need a long line with significant spaces.
But in the current case, current strategy was perfectly sufficient for
functionnal needs IMHO

regards,

Hervé

>
> Cheers,
> -Lukas
>
> PS Are you saying you are using site-plugin beta-6 with doxia beta-1? I
> haven't been able to make the two work together yet...
>
> Hervé BOUTEMY wrote:
> > Le mercredi 28 novembre 2007, Lukas Theussl a écrit :
> >>Herve,
> >>
> >>Thanks for fixing this! Just two questions:
> >>
> >>Why do we need a separate method in AbstractXmlSink, can't we just
> >>remove the EOL from writeEndTag()?
> >
> > I tried to do so in the first place: that's exactly what I wrote when
> > mailing to maven-dev.
> > But when doing so, I discovered 2 drawbacks:
> > - the whole generated HTML document was without any EOL, which is quite
> > hard to read. But no major problem here, I just thought that people who
> > had written these newlines prefer readable HTML, and wouldn't be happy if
> > I changed their "way of code" :)
> > - unit test were failing, since some sinks use LineBreaker class which
> > adds a newline when larger than 78 chars: the whole unit tests logic had
> > to be reworked...
> > Then I discovered where really these newlines were a problem, and where
> > they were not: see next paragraph...
> >
> >>And what's the reason for selecting only special tags to write no
> >>newline? Just because they are inline elements? This doesn't solve the
> >>issue of EOLs within <pre> tags, right?
> >
> > Yes, because they're inline elements: HTML <a>, <i>, <b> and <code>.
> > Within <pre> tag, you don't use other tags, which are structural tags:
> > you only use inline elements for styling and links/anchors.
> > FYI, here is a page on which I discovered the problem when using
> > 2.0-beta6 http://logdistiller.sourceforge.net/quickstart-4.html
> >
> > Feel free to check and comment this logic, because I think it is good but
> > I'm not 100% sure I didn't miss a special case...
> >
> > regards
> >
> > Hervé
> >
> >>Cheers,
> >>-Lukas

[Hervé BOUTEMY]

Le jeudi 29 novembre 2007, Lukas Theussl a écrit :

> Hi,
>
> We had a discussion about EOLs some time ago, I would like to know which
> tests failed for you. The AbstractSinkTest (which all sink tests are
> supposed to extend) filters out every EOL in the strings it compares (my
> reasoning for that was that the case where EOLs are significant is
> usually a special case and should be tested separately).
>
> Apart from that, IMO AbstractXmlSink is an abstract base class that
> shouldn't be concerned with formatting (btw, I just noticed that
> writeStartTag also appends an EOL after simple tags, that should be
> removed as well). The writeEndTagWithoutEOL() method is simply not
> needed because implementing sinks should write the EOLs where they need
> them. In the current case, it should be enough to move the EOLs into the
> XhtmlBaseSink.
>
> If tests fail after that then it's probaby the tests that need to be
> adapted...
>
> Cheers,
> -Lukas
>
> PS Are you saying you are using site-plugin beta-6 with doxia beta-1? I
> haven't been able to make the two work together yet...

I forgot: no, I didn't test the full stack Doxia + maven-site-plugin
I just found in Doxia unit tests the cause of the problem

>
> Hervé BOUTEMY wrote:
> > Le mercredi 28 novembre 2007, Lukas Theussl a écrit :
> >>Herve,
> >>
> >>Thanks for fixing this! Just two questions:
> >>
> >>Why do we need a separate method in AbstractXmlSink, can't we just
> >>remove the EOL from writeEndTag()?
> >
> > I tried to do so in the first place: that's exactly what I wrote when
> > mailing to maven-dev.
> > But when doing so, I discovered 2 drawbacks:
> > - the whole generated HTML document was without any EOL, which is quite
> > hard to read. But no major problem here, I just thought that people who
> > had written these newlines prefer readable HTML, and wouldn't be happy if
> > I changed their "way of code" :)
> > - unit test were failing, since some sinks use LineBreaker class which
> > adds a newline when larger than 78 chars: the whole unit tests logic had
> > to be reworked...
> > Then I discovered where really these newlines were a problem, and where
> > they were not: see next paragraph...
> >
> >>And what's the reason for selecting only special tags to write no
> >>newline? Just because they are inline elements? This doesn't solve the
> >>issue of EOLs within <pre> tags, right?
> >
> > Yes, because they're inline elements: HTML <a>, <i>, <b> and <code>.
> > Within <pre> tag, you don't use other tags, which are structural tags:
> > you only use inline elements for styling and links/anchors.
> > FYI, here is a page on which I discovered the problem when using
> > 2.0-beta6 http://logdistiller.sourceforge.net/quickstart-4.html
> >
> > Feel free to check and comment this logic, because I think it is good but
> > I'm not 100% sure I didn't miss a special case...
> >
> > regards
> >
> > Hervé
> >
> >>Cheers,
> >>-Lukas