10 Jul 2009

On Jul 4, 2009, at 2:44 PM, tmortagne (SVN) wrote:

...
  Author: tmortagne
 Date: 2009-07-04 14:44:30 +0200 (Sat, 04 Jul 2009)
 New Revision: 21811

 Modified:
   platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 Document.java
   platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 XWiki.java
   platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/ 
 XWikiDocument.java
   platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/api/ 
 XWikiTest.java
   platform/core/trunk/xwiki-rendering/xwiki-rendering-api/pom.xml
   platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/internal/renderer/ 
 DefaultPrintRendererFactory.java
   platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/parser/Syntax.java
   platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/parser/SyntaxType.java
   platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/renderer/PrintRendererFactory.java
   platform/web/trunk/standard/src/main/webapp/templates/contentview.vm
   platform/web/trunk/standard/src/main/webapp/templates/plain.vm
 Log:
 XWIKI-4042: Add a way to choose the renderer when viewing a page 
...

 Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ 
 api/Document.java
 ===================================================================
 --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 Document.java	2009-07-04 12:42:14 UTC (rev 21810)
 +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 Document.java	2009-07-04 12:44:30 UTC (rev 21811)
 @@ -37,6 +37,7 @@
 import org.suigeneris.jrcs.diff.DifferentiationFailedException;
 import org.suigeneris.jrcs.diff.delta.Delta;
 import org.suigeneris.jrcs.rcs.Version;
 +import org.xwiki.rendering.parser.Syntax;

 import com.xpn.xwiki.XWiki;
 import com.xpn.xwiki.XWikiContext;
 @@ -473,6 +474,16 @@
     }

     /**
 +     * @param targetSyntax the syntax in which render the document  
 content
 +     * @return the rendered content
 +     * @throws XWikiException error when rendering content
 +     */
 +    public String getRenderedContent(Syntax targetSyntax) throws  
 XWikiException
 +    {
 +        return this.doc.getRenderedContent(targetSyntax,  
 getXWikiContext());
 +    } 
Since this is for Velocity, isn't it better to use a syntax Id String  
instead of a Syntax object which is hard to construct from velocity?

...
  +
 +    /**
      * return a escaped version of the content of this document
      */
     public String getEscapedContent() throws XWikiException

 Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ 
 api/XWiki.java
 ===================================================================
 --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 XWiki.java	2009-07-04 12:42:14 UTC (rev 21810)
 +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ 
 XWiki.java	2009-07-04 12:44:30 UTC (rev 21811)
 @@ -30,6 +30,8 @@
 import org.apache.commons.logging.LogFactory;
 import org.suigeneris.jrcs.diff.delta.Chunk;
 import org.xwiki.query.QueryManager;
 +import org.xwiki.rendering.parser.Syntax;
 +import org.xwiki.rendering.renderer.PrintRendererFactory;

 import com.xpn.xwiki.XWikiContext;
 import com.xpn.xwiki.XWikiException;
 @@ -2685,4 +2687,42 @@
     {
         return this.xwiki.getDefaultDocumentSyntax();
     }
 +
 +    /**
 +     * Find the corresponding available renderer syntax.
 +     * <p>
 +     * If <code>syntaxVersion</code> is null the last version of  
 the available provided syntax type is returned.
 +     *
 +     * @param syntaxType the syntax type
 +     * @param syntaxVersion the syntax version
 +     * @return the available corresponding {@link Syntax}. Null if  
 no available renderer can be found.
 +     */
 +    public Syntax getAvailableRendererSyntax(String syntaxType,  
 String syntaxVersion)
 +    {
 +        Syntax syntax = null;
 +
 +        PrintRendererFactory printRendererFactory =
 +            (PrintRendererFactory)  
 Utils.getComponent(PrintRendererFactory.class);
 +
 +        List<Syntax> availableSyntaxes =  
 printRendererFactory.getAvailableSyntaxes(); 
We already have the notion of available syntaxes in the SyntaxFactory.  
I don't understand why we need to have them in the  
PrintRendererFactory (which is not supposed to return syntaxes BTW,  
it's only supposed to return PrintRenderer(s).

...
  +
 +        for (Syntax availableSyntax : availableSyntaxes) { 
Why not instead do:
SyntaxType syntaxType = SyntaxType.getSyntaxType(syntaxTypeAsString);

And then (when syntaxVersion != null):
Syntax syntax = new Syntax(syntaxType, syntaxVersion);

And then:
syntaxFatory.getAvailableSyntaxes().contains(syntax)

hmmm.... Actually thinking more about it, I believe this whole code  
should be moved out of XWikiDocument and instead go in the rendering  
module in SyntaxFactory.

...
  +            if (syntaxVersion != null) {
 +                if  
 (availableSyntax.getType().toIdString().equalsIgnoreCase(syntaxType)
 +                    &&  
 availableSyntax.getVersion().equals(syntaxVersion)) {
 +                    syntax = availableSyntax;
 +                    break;
 +                }
 +            } else {
 +                // TODO: improve version comparaison since it does  
 not work when comparing 2.0 and 10.0 for example. We
 +                // should have a Version which implements  
 Comparable like we have SyntaxId in Syntax
 +                if  
 (availableSyntax.getType().toIdString().equalsIgnoreCase(syntaxType)
 +                    && (syntax == null ||  
 availableSyntax.getVersion().compareTo(syntax.getVersion()) > 0)) {
 +                    syntax = availableSyntax;
 +                }
 +            }
 +        }
 +
 +        return syntax;
 +    }
 } 
[snip]
...

     /**
 +     * @return the syntax of the document
 +     */
 +    private Syntax getSyntax()
 +    {
 +        Syntax syntax = null;
 +
 +        String syntaxId = getDefaultDocumentSyntax();
 +        try {
 +            syntax =  
 this.syntaxFactory.createSyntaxFromIdString(getSyntaxId()); 
Shouldn't we save the Syntax object instead of syntaxId in  
XWikiDocument so that we don't perform the conversion every time  
getSyntax is called?

...
  +        } catch (ParseException e) {
 +            LOG.error("Failed to genrate Syntax object for syntax  
 identifier [" + syntaxId + "] in page [" 
Typo: generate

...
  +                + getDocumentName() + "]",
e);
 +
 +            syntaxId = getDefaultDocumentSyntax(); 
I don't understand this line above, it's the same as the one a few  
lines above.

...
  +            try {
 +                syntax =  
 this 
 .syntaxFactory.createSyntaxFromIdString(getDefaultDocumentSyntax()); 
It seems you're not using the syntaxId variable.

...
  +            } catch (ParseException e1) {
 +                LOG.error("Failed to genrate default Syntax object.  
 The defautlt syntax id in [" + syntaxId + "]", e); 
same typo

[snip]

...
  Modified:
platform/core/trunk/xwiki-rendering/xwiki-rendering-api/ 
 pom.xml
 ===================================================================
 --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/pom.xml	 
 2009-07-04 12:42:14 UTC (rev 21810)
 +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/pom.xml	 
 2009-07-04 12:44:30 UTC (rev 21811)
 @@ -187,7 +187,9 @@
                 <!-- Excludes for the 2.0 M2 release. Once it's  
 released remove them. -->
                 <exclude>org/xwiki/rendering/macro/MacroManager</ 
 exclude>
                 <exclude>org/xwiki/rendering/macro/MacroSource</ 
 exclude>
 -                <exclude>org/xwiki/rendering/macro/ 
 AbstractMacroSource</exclude>
 +                <exclude>org/xwiki/rendering/macro/ 
 AbstractMacroSource</exclude>
 +                <!-- Seems clirr plugin consider adding new API as  
 an error (which I don't understand). --> 
It doesn't normally (it' considered as an INFO level) unless it breaks  
binary compatibility. But I don't know why in your case (would need to  
check more the code, i'm only looking at the svn diff).

...
  +               
<exclude>org/xwiki/rendering/renderer/ 
 PrintRendererFactory</exclude>
               </excludes>
             </configuration>
           </execution>

 Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/ 
 src/main/java/org/xwiki/rendering/internal/renderer/ 
 DefaultPrintRendererFactory.java
 ===================================================================
 --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/internal/renderer/ 
 DefaultPrintRendererFactory.java	2009-07-04 12:42:14 UTC (rev 21810)
 +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/ 
 java/org/xwiki/rendering/internal/renderer/ 
 DefaultPrintRendererFactory.java	2009-07-04 12:44:30 UTC (rev 21811)
 @@ -19,6 +19,10 @@
  */
 package org.xwiki.rendering.internal.renderer;

 +import java.util.Arrays;
 +import java.util.Collections;
 +import java.util.List;
 +
 import org.xwiki.component.annotation.Component;
 import org.xwiki.component.annotation.Requirement;
 import org.xwiki.rendering.parser.Syntax;
 @@ -43,6 +47,10 @@
 @Component
 public class DefaultPrintRendererFactory implements  
 PrintRendererFactory
 {
 +    private static final List<Syntax> AVAILABLE_SYNTAXES =
 +         
 Collections.unmodifiableList(Arrays.asList(Syntax.XHTML_1_0,  
 Syntax.XWIKI_2_0, Syntax.EVENT_1_0,
 +            Syntax.TEX_1_0, Syntax.PLAIN_1_0)); 
I don't like this, see above.

...
  +
     /**
      * Factory to easily create an XHTML Image and Link Renderers.
      */
 @@ -55,6 +63,16 @@
     /**
      * {@inheritDoc}
      *
 +     * @see  
 org 
 .xwiki.rendering.renderer.PrintRendererFactory#getAvailableSyntaxes()
 +     */
 +    public List<Syntax> getAvailableSyntaxes()
 +    {
 +        return AVAILABLE_SYNTAXES;
 +    }
 + 
I don't like this, see above.

[snip]

Thanks
-Vincent