Sergiu Dumitriu wrote:
> Florin Ciubotaru wrote:
>
>> 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?
>>
>
> I don't know about other, but I do it 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?
>>
>
> This one starting here:
>
Yes, I actually saw it. Just hoped I don't need to change it by
hand(used with VS that usually does it all).
Hopefully I'll drop this soon.
>
>>>
>>>
>>>> +<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>
>>>>
>
> and ending here.
>
>
>>>> +<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.
>>
>
> OK. Fine by me.
>
>
>>>
>>>
>>>> +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.
>>
>
> I usually stay out of early projects, since the code tends to move
> pretty fast, and in pretty big chunks, thus it's hard to track. As time
> allows, I'll do more code reviews from now on.
>
>
That would be great. Thanks :)
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
