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

_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to