[Development] QTimeZone available for review
markg85 at gmail.com
Thu Mar 7 13:46:03 CET 2013
On Thu, Mar 7, 2013 at 11:10 AM, John Layt <jlayt at kde.org> wrote:
> 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
> QDateTime - Improve and expose Qt::OffsetFromUtc
> QDateTime - Extend fromMSecsSinceEpoch API
> QTimeZone - Define new class and api
> QTimeZone - Add ICU support
> QTimeZone - Add TZ File Backend
> QTimeZone - Add Mac backend
> QTimeZone - Add Windows backend
> WIP QDateTime - Add QTimeZone support
> 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
> * 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.
Let me just congratulate you on this quite impressive milestone.
Congrats and keep up the awesome work!
More information about the Development