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

[Sebastian Trüg]

On Friday 16 March 2007 13:51:16 David Jarvie wrote:
> 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.

Sorry, I go that mixed up, time-only is not parsed. My bad. Of course you are 
right.

> >> 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.

right. I did not think of that.

> >> 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.

you are right.

> >> 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.

Well, I think I will have to look into that timezone issue properly again. I 
had hoped that this could be a quick fix... ;)

> >> 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.