Sergiu Dumitriu wrote:
> 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...
>
Didn't pay attention to this until now. I usually export without
history, does that fix the problem? Or do we need to change this by hand?
>
>> +<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.
>
Which tag?
>
>> +<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.
>
I'm -1 on adding a license header to every file while developing .net.
There are a lot of auto-generated source files that cannot contain it.
Also, a class can be written across multiple files and a file can
contain multiple classes. A class can have multiple vendors that package
it in one or multiple assemblies dependent or independent from one another.
It's hard to keep a consistency, this is why the convention I use is to
have only one license header in the add-ins entry point file.
If other add-ins will be created then each one will have it's license
header in the entry point file, not matter if they will be a part of the
same .net solution or not. But until then, there will be only one
license file.
>
>> +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.
>
Yes, that was supposed to be a factory, but this entire thing about
using wiki pages as services is wrong, so it will be dropped. It's
already around a lot more then planned.
>
>> /// <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...
>
Bad naming of a property made by a java programmer. Sorry, I didn't
notice it when I reviewed the patch. I'll fix it in one of the next
commits. The correct name is just "Encoding".
>
>> + set { getEncoding = value; }
>> + }
>> +
>> }
>> }
>>
>
>
This is the first code review XWord had in months of develpement. The more the
better.
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
