fciubotaru (SVN) wrote:
> Author: fciubotaru
> Date: 2009-04-13 21:19:15 +0200 (Mon, 13 Apr 2009)
> New Revision: 18638
> 
> Added:
>    xoffice/trunk/xword/Wiki/src/main/resources/MSOffice/GetEncodingService.xml
>    xoffice/trunk/xword/XWikiLib/Clients/InvalidEncodingNameException.cs
> Modified:
>    xoffice/trunk/xword/XWikiLib/Clients/IXWikiClient.cs
>    xoffice/trunk/xword/XWikiLib/Clients/XWikiHTTPClient.cs
>    xoffice/trunk/xword/XWikiLib/Clients/XWikiXMLRPCClient.cs
>    xoffice/trunk/xword/XWikiLib/XWiki/XWikiURLFactory.cs
>    xoffice/trunk/xword/XWikiLib/XWikiLib.csproj
>    xoffice/trunk/xword/XWord/AddinActions.cs
> Log:
> XOFFICE-39 Save and send data using the encoding of the wiki.
> Patch submitted by Cristina Scheau, reviewed by Florin Ciubotaru.
> 
> Added: 
> xoffice/trunk/xword/Wiki/src/main/resources/MSOffice/GetEncodingService.xml
> ===================================================================
> --- 
> xoffice/trunk/xword/Wiki/src/main/resources/MSOffice/GetEncodingService.xml   
>                             (rev 0)
> +++ 
> xoffice/trunk/xword/Wiki/src/main/resources/MSOffice/GetEncodingService.xml   
>     2009-04-13 19:19:15 UTC (rev 18638)
> @@ -0,0 +1,61 @@
> +<?xml version="1.0" encoding="ISO-8859-15"?>
> +
> +<xwikidoc>
> +<web>MSOffice</web>
> +<name>GetEncodingService</name>
> +<language></language>
> +<defaultLanguage>en</defaultLanguage>
> +<translation>0</translation>
> +<parent></parent>
> +<creator>XWiki.Admin</creator>
> +<author>XWiki.Admin</author>
> +<customClass></customClass>
> +<contentAuthor>XWiki.Admin</contentAuthor>
> +<creationDate>1239491696000</creationDate>
> +<date>1239493018000</date>
> +<contentUpdateDate>1239493018000</contentUpdateDate>
> +<version>2.1</version>

Normally we set the document version to 1.1...

> +<title>GetEncodingsService</title>
> +<template></template>
> +<defaultTemplate></defaultTemplate>
> +<validationScript></validationScript>
> +<comment></comment>
> +<minorEdit>false</minorEdit>
> +<syntaxId>xwiki/1.0</syntaxId>
> +<hidden>false</hidden>

... and we remove the tag object when committing wiki documents.

> +<object>
> +<class>
> +<name>XWiki.TagClass</name>
> +<customClass></customClass>
> +<customMapping></customMapping>
> +<defaultViewSheet></defaultViewSheet>
> +<defaultEditSheet></defaultEditSheet>
> +<defaultWeb></defaultWeb>
> +<nameField></nameField>
> +<validationScript></validationScript>
> +<tags>
> +<cache>0</cache>
> +<displayType>input</displayType>
> +<multiSelect>1</multiSelect>
> +<name>tags</name>
> +<number>1</number>
> +<prettyName>Tags</prettyName>
> +<relationalStorage>1</relationalStorage>
> +<separator> </separator>
> +<separators> ,|</separators>
> +<size>30</size>
> +<unmodifiable>0</unmodifiable>
> +<values></values>
> +<classType>com.xpn.xwiki.objects.classes.StaticListClass</classType>
> +</tags>
> +</class>
> +<name>MSOffice.GetEncodingService</name>
> +<number>0</number>
> +<className>XWiki.TagClass</className>
> +<guid>9fd947e4-3ebe-41b0-ba90-2cd80b3c7df3</guid>
> +<property>
> +<tags/>
> +</property>
> +</object>
> +<content>$xwiki.getEncoding()</content>
> +</xwikidoc>
> \ No newline at end of file
> 
> Added: xoffice/trunk/xword/XWikiLib/Clients/InvalidEncodingNameException.cs
> ===================================================================
> --- xoffice/trunk/xword/XWikiLib/Clients/InvalidEncodingNameException.cs      
>                         (rev 0)
> +++ xoffice/trunk/xword/XWikiLib/Clients/InvalidEncodingNameException.cs      
> 2009-04-13 19:19:15 UTC (rev 18638)
> @@ -0,0 +1,11 @@

Shouldn't there be a license header in here? I've looked, and it seems 
that most XWord source files don't have such a header, which is wrong.

> +using System;
> +using System.Collections.Generic;
> +using System.Linq;
> +using System.Text;
> +
> +namespace XWiki.Clients
> +{
> +    class InvalidEncodingNameException : Exception
> +    {
> +    }
> +}
> 
> Modified: xoffice/trunk/xword/XWikiLib/XWiki/XWikiURLFactory.cs
> ===================================================================
> --- xoffice/trunk/xword/XWikiLib/XWiki/XWikiURLFactory.cs     2009-04-13 
> 18:25:16 UTC (rev 18637)
> +++ xoffice/trunk/xword/XWikiLib/XWiki/XWikiURLFactory.cs     2009-04-13 
> 19:19:15 UTC (rev 18638)
> @@ -16,6 +16,7 @@
>          static String wikiStructureURL = 
> "/xwiki/bin/view/MSOffice/StructureService?xpage=plain";
>          static String attachmentServiceURL = 
> "/xwiki/bin/view/MSOffice/AttachmentService?xpage=plain";
>          static String protectedPagesURL = 
> "/xwiki/bin/view/MSOffice/ProtectedPages?xpage=plain";
> +        static String getEncoding = 
> "/xwiki/bin/view/MSOffice/GetEncodingService?xpage=plain";

I don't like this. URLs are pretty configurable, and this hardcoding 
makes it impossible for XWord to work with a non-default installation of 
XWiki. There should be a configurable prefix, which is used to construct 
the final URLs.

I've also looked at this class as a whole, and it doesn't look like a 
Factory. Using the wrong name for a thing is bad, since somebody new to 
the code will waste important time trying to figure out why he doesn't 
see the factory in there. If this class will change in the future (once 
the communication mechanism is replaced) to behave like a factory, then 
you should write a class comment stating that although it doesn't look 
like a factory yet, it will soon be one.

>          /// <summary>
>          /// Gets or sets the URL of the service that handles attachments.
> @@ -72,5 +73,15 @@
>              get { return protectedPagesURL; }
>              set { protectedPagesURL = value; }
>          }
> +
> +        /// <summary>
> +        /// Gets the encoding of the wiki.
> +        /// </summary>
> +        public static String GetEncoding
> +        {
> +            get { return getEncoding; }

Can you set a Get* member? Weird...

> +            set { getEncoding = value; }
> +        }
> +
>      }
>  }


-- 
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to