[Development] QTimeZone available for review
John Layt
jlayt at kde.org
Thu Mar 7 11:10:31 CET 2013
Hi,
I've finally pushed my proposed QTimeZone support to Gerrit for initial review
for possible inclusion in 5.1.
The reviews are:
QLocale - Add private countryToCode() method
https://codereview.qt-project.org/50064
QDateTime - Improve and expose Qt::OffsetFromUtc
https://codereview.qt-project.org/50065
QDateTime - Extend fromMSecsSinceEpoch API
https://codereview.qt-project.org/50066
QTimeZone - Define new class and api
https://codereview.qt-project.org/50067
QTimeZone - Add ICU support
https://codereview.qt-project.org/50068
QTimeZone - Add TZ File Backend
https://codereview.qt-project.org/50069
QTimeZone - Add Mac backend
https://codereview.qt-project.org/50070
QTimeZone - Add Windows backend
https://codereview.qt-project.org/50071
WIP QDateTime - Add QTimeZone support
https://codereview.qt-project.org/50072
I'd appreciate particular attention to the API review, and the Mac and Windows
backends which are not my usual dev environment.
What remains to be done:
* The QDateTime integration is not quite finished and so marked as WIP, but
the API changes need review.
* The Windows backend can have support for historic data added from Vista
onwards.
* Lots more test cases
* A decision on parse/format (see below)
One big change since I first posted the code is there is now no "system time
zone" that tracks the underlying system time zone as we already provide this
facility in QDateTime using Qt::LocalTime.
The main issue outstanding is parse/format. Our current support for time
zones is inconsistent, asymmetrical, and broken in places. You can format
time zones in QLocale and QTime (albeit often the wrong name) but not
QDateTime, and not parse them anywhere, so round-trip using LongFormat is
currently broken. The existing code is messy, and while format is fairly
easy to make work, parsing is near impossible to support in our current code.
It would probably require major changes to the complex and fragile
QDateTimeParser and QDateTimeEdit classes, but would still be deeply ambiguous
due to our only supporting a format code for abbreviations. Full support
would require either breaking the current behaviour or implementing all new
code and api names, which seems pointless for just 5.1 when I hope to have the
new ICU based parser ready for 5.2. Instead I propose to:
* Remove the QDateTime formatter which doesn't support tz and use the QLocale
formatter instead which does support tz
* Modify the QLocale formatter to properly format time zones, including the
Win/Mac system locales
* To not support parsing of time zones in the old code and clearly document as
such, recommending apps use a combo-box instead, or use the Qt::ISODate which
does support parsing of an offset.
* To leave parsing support until 5.2 using ICU
I realise that might be controversial, and I have had some feedback that this
is a blocker issue, but I feel the benefits of having most of the Time Zone
support in 5.1 far out-weighs the few use-cases requiring parsing, especially
as the existing behaviour is already asymmetrical and broken.
Cheers!
John.
More information about the Development
mailing list