PATCH: time-only support for KDateTime (used in Nepomuk-KDE)

[David Jarvie]

On Friday 16 March 2007 10:52, Sebastian Trüg wrote:
> On Thursday 15 March 2007 14:12:40 David Jarvie wrote:
>> On Thursday 15 March 2007 12:02, Sebastian Trüg wrote:
>> I won't be able to try it out until the weekend, but AFAICR date-only
>> strings like YYYY-MM-DD *are* handled, provided no time zone or time
>
> well, they are not parsed without my patch.

The unit test checks that YYYY-MM-DD conversions work for ClockTime
date-only values. For non-ClockTime values, a UTC offset has to be
specified in the string, so YYYY-MM-DD isn't a proper representation in
those cases.

>> offset is specified. My reading of ISO8601 is that if a time zone
>> specification/offset is included in the string, a time value must also
>> be
>> included. That's why a time of 00:00:00 is appended to the date for a
>> date-only value except when it's ClockTime. Unless I've misunderstood
>> the
>> standard, I don't think your change is correct.
>
> hmm, I don't really get the use of a timezone for a date-only instance
> anyway.
> Anyway, that would mean that I just have to use ClockTime date-only
> instances
> in Nepomuk-KDE. That seems reasonable.
> I attached a new patch that changes it back to the original behaviour.

Time zones/offsets actually do make sense for date-only values. For
example KDateTime allows you to compare a date-only value 2001-1-1 with a
UTC offset of +5 hours with a date/time value of 2001-1-2, UTC offset -8
hours, to see whether the latter falls within the former, or whether it
falls before or after.

>> I'm doubtful about the treatment of time-only values which have a
>> TimeZone
>> time specification. Without an associated date, a time value is
>> ambiguous
>> if a time zone is applied. (Time zones can adjust the meaning of a time
>> by
>> up to 2 hours depending on the date in the year. And the meaning is even
>> more ambiguous for years before or after the validity of the time zone
>> data.) Because of this, I suggest that a time zone specification should
>> be
>> treated the same as ClockTime. I would still allow a time zone to be
>> specified for record purposes, but it should be disregarded in
>> calculations.
>
> Why not just inform the user (via documentation) that calculations with a
> timezone can never be 100% accurate. Because there may be many situations
> in
> which it makes sense to have time-only values with a time-zone.

If time zone offsets are to be used, there needs to be a clear definition
in the apidox as to how KDateTime determines what UTC offset to use for
any given time zone. Your patch looks as if it will use the offset for
1/1/0001. Although time zone definitions do normally define a UTC offset
for dates before the start of their data, I'm not convinced that using a
date long before time zones were invented is the best default. I'd suggest
perhaps using the non-DST offset for the current year. The side effect of
this would be to complicate any calculations based on using 'sot' as the
date for a time-only value, so it might be necessary to rethink the use of
'sot'. (See my comments below about the setTimeOnly() method, for
example.)

>> When calculations are performed on time-only values, the result should
>> always be either a time-only value or invalid. AFAICS, the necessary
>> mods
>> to the code to achieve this haven't been done. For example, in
>> KDateTime::addSecs(), it doesn't look as if a time-only value is
>> returned
>> when 'this' is time-only, and if the time addition happened to wrap
>> round
>> midnight, even the date wouldn't be sot.
>
> I fixed that and also the other addXXX methods. I hope that I did not miss
> anything. I did not write time-only unit tests yet though.

To return time-only values, I think it would be clearer and neater to use
your new time-only constructor than using a date-time constructor with an
invalid date.

>> In KDateTimePrivate::setTimeOnly(), mDt should be set using setDt() to
>> ensure that 'ut' and caching members are updated properly.
>
> done.

Sorry - I didn't think sufficiently before writing the above comment. You
should really try to preserve any valid caching that's been done already
on the value. So you need to take account of whether it's a UTC value or
what time zone it is, etc. A simple call to setDt() wipes out anything
already cached, while setting mDt directly may or may not invalidate the
cache. Plus, you need to take account for a TimeZone instance what date is
being used as the basis for evaluating the UTC offset. That could make
things altogether more complicated. Also remember that QDateTime in Qt 4
has an important third parameter - don't use the default without being
sure it's appropriate.

>> In kdatetime.h, the new %f format specification should be described
>> sufficiently to not need to look up an external reference to find out
>> what
>> output it produces. By all means include the reference, but also explain
>> its effects.
>
> also done. I hope an example is enough.

It's clear now. Thanks.

-- 
David Jarvie.
KAlarm author & maintainer.
http://www.astrojar.org.uk/kalarm