Review Request: Switch implementation of formatDate() and readDate() from KLocale to KCalendarSystem.

[John Layt]

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

(Updated 2009-08-17 16:11:02.234419)


Review request for kdelibs.


Changes
-------

Bump.  Any comments?  I need to commit this so I can put my Plasma Clock/Calendar changes up for review.  If no response I'll commit Tuesday.


Summary (updated)
-------

[Bump.  Any comments?  I need to commit this so I can put my Plasma Clock/Calendar changes up for review.  If no response I'll commit Tuesday.]

This review request is more for the refactoring model to see if there is a scenario I have missed, rather than for the code which is mostly a cleaned-up copy-and-paste.

Currently the KCalendarSystem implementations of formatDate() and readDate() point to the KLocale implementations.  The problem with this is that the KLocale implementations in turn point to the global calendar when obtaining date components like month name or year.  This is fine when you only ever show the global calendar, but if you want to show a different calendar at the same time, e.g. the Hebrew calendar alongside the global Gregorian, then calling KCalendarSystem::formatDate() on the Hebrew calendar returns the Gregorian date and not the Hebrew date as expected.  There is a workaround to this by creating a new KLocale instance, creating the new KCalendarSystem using this and pointing the KLocale back at the new KCalendarSystem.  This however is an ugly hack and we shouldn't expect the app programmer to know to do this.  The app programmer would expect they can create a new KCalendarSystem instance and have the correct dates returned.  Only if they want dates formatted in a different format than the global format should they have to create a new KLocale first.

The solution is to reverse the situation by moving the formatDate() and readDate() implementations from KLocale into KCalendarSystem, with the KLocale implementations pointing to its internal calendar instance for formatDate() and readDate(), and KCalendarSystem using its correct internal date component routines and pointing back to KLocale only to get the layout formats.  Another advantage to this model is that any calendar system that needs non-standard date formatting rules can now override the formatDate() directly without adding complexity to the standard routines.

A couple of code clean-ups and bug-fixes are included, especially around FancyDate format and date validity checking.


Diffs
-----

  trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h 1010188 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.cpp 1010188 
  trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhebrew.cpp 1010188 
  trunk/KDE/kdelibs/kdecore/localization/klocale.cpp 1010188 

Diff: http://reviewboard.kde.org/r/1285/diff


Testing
-------

All current klocaletest unit test cases for formatDaet(), formatDateTime(), readDate(), and readDateTime() pass the same as they did before the changes.

I have implemented support in the plasma calendar widget to allow the user to select what calendar system to display.  Without this change the wrong dates are displayed/read.  With these changes the correct dates are displayed/read.


Thanks,

John