Review Request: fix writing of text:time values

[Pierre Stirnweiss]

> On July 7, 2012, 4:50 p.m., Pierre Stirnweiss wrote:
> > plugins/variables/DateVariable.cpp, line 126
> > <http://git.reviewboard.kde.org/r/105463/diff/3/?file=71690#file71690line126>
> >
> >     Won't this create a round-trip problem?
> >     The loaded file haven't got a date. On saving it will have currentDate, or have I missed something?
> 
> Friedrich W. H. Kossebau wrote:
>     It will, but it should not save the date, only the time. That is the idea of that m_valueType (which really needs a better name obviously, because you missed that one :) ).
>     If m_valueType is Single, it should hint to write just the date or just the time, if Combined, a datetime should be saved.
>     Do you still see a problem with that?
>     
>     And thanks for reviewing :)
>

Oups, silly me. No I don't have a problem with this.
Perhaps a little comment on that line, so that in the future, if some silly like me come across it, they'll know ;)


- Pierre


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105463/#review15518
-----------------------------------------------------------


On July 7, 2012, 10:16 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105463/
> -----------------------------------------------------------
> 
> (Updated July 7, 2012, 10:16 a.m.)
> 
> 
> Review request for Calligra and Thorsten Zachmann.
> 
> 
> Description
> -------
> 
> The file interoperability/wordprocessing/oowriter/oow_insert_fields.odt from calligratests currently fails on this bad value:
> text:time-value="0-00-00T07:37:38"
> 
> Reason is that ODF 19.881 text:time-value specifies
> "The text:time-value attribute has the data type timeOrDateTime."
> and ODF 18.3.36 timeOrDateTime says
> "A timeOrDateTime value is either an [xmlschema-2] time or an [xmlschema-2] dateTime value."
> 
> Now, [xmlschema-2] demands for the year part a "four-or-more digit optionally negative-signed numeral that represents the year". Also, "There is no year 0, and '0000' is not a valid lexical representation.", see http://www.w3.org/TR/xmlschema-2/#year-zero
> 
> Thus the validator fails on this (good boy).
> 
> Looking more at the code (also after sending a premature patch here) I see the DateVariable supports both time and date variables (or a combination).
> Just, the storage format for both date or time can be simple (just date or time) or combined (datetime). As this info makes a difference (e.g. having just a time without date (so relative) is different from a given datetime (which is absolute)), it should be at least supported in the save and load code, even if the UI does not yet expose all that.
> 
> Attached patch fixes the code to do that, both the loading/saving of the complete data (without adding/removing info) as well as the broken "0-00-00T" stuff.
> 
> I am not satisfied with the names of the new enum ValueType, proposals welcome.
> 
> Okay to backport to 2.5?
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoOdfWorkaround.h 8672f22 
>   libs/flake/KoOdfWorkaround.cpp 8d3d523 
>   plugins/variables/DateVariable.h 9d06879 
>   plugins/variables/DateVariable.cpp cf5ef8b 
> 
> Diff: http://git.reviewboard.kde.org/r/105463/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120707/4360f11a/attachment.htm>