Review Request: KLocale / KCalendarSystem: Add new enum for CalendarSystem

[John Layt]

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

Review request for kdelibs.


Summary
-------

This is related to the KLocalizedDate review 5692.  I realised in that review that KLocalizedDate should now be the only api people use to localize dates, and so rather than exactly matching the old KCalendarSystem api I could make some changes to simplify / improve the api.  (The bits matching the QDate api won't change).

As part of this, I want to introduce an enum for CalendarSystem to replace the current QString Calendar Type, e.g. replace "hebrew" with KLocale::HebrewCalendar.  This is obviously clearer, safer and less error prone.

Note that I had to put the enum in KLocale rather than KCalendarSystem as the KCalendarSystem api already uses KLocale enums in the header, so I can't use any KCalendarSystem enums in the KLocale api and header (you can't forward-declare enums).

The awkward part was naming the enums, particularly the default QDate hybrid Julian/Gregorian system.  I chose to call this KLocale::QDateCalendar rather than KLocale::GregorianCalendar which I've used for the proper one.  Any better suggestions welcome.  I've also renamed the Hijri to IslamicCivilCalendar to distinguish from the lunar calendar.  I'll probably rename the derived calendar classes to match the enum, which should be BC safe as they are not exported, but I'll do that later to save cluttering up the diff. [Random thought: do we even need the private classes derived from the base class anymore, the derived private d_ptr class should be enough?]

You probably don't want to look too closely at the KCalendarSystem spaghetti, you'll go cross-eyed, the KLocale is of more interest :-)


Diffs
-----

  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemcoptic.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemcopticprivate_p.h 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemethiopian.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorian.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorianproleptic.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorianprolepticprivate_p.h 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhebrew.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemhijri.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemindiannational.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjalali.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjapanese.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemjulian.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemminguo.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemprivate_p.h 1190608 
  /trunk/KDE/kdelibs/kdecore/date/kcalendarsystemthai.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/localization/klocale.h 1190608 
  /trunk/KDE/kdelibs/kdecore/localization/klocale.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/localization/klocale_kde.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/localization/klocale_p.h 1190608 
  /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h 1190608 
  /trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp 1190608 
  /trunk/KDE/kdelibs/kdecore/tests/klocaletest.h 1190608 
  /trunk/KDE/kdelibs/kdecore/tests/klocaletest.cpp 1190608 

Diff: http://svn.reviewboard.kde.org/r/5704/diff


Testing
-------

All existing unit tests pass, new unit tests written.  (Note, ignore the KLocalizedDate tests below, it's just the overlap from the other review, they will be committed separately.)


Thanks,

John

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20101028/db6e1ffb/attachment.htm>