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

[Sebastian Trueg]

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



/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h
<http://svn.reviewboard.kde.org/r/5704/#comment8734>

    add a hint about what should be used instead.



/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h
<http://svn.reviewboard.kde.org/r/5704/#comment8735>

    add a hint about what should be used instead.



/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h
<http://svn.reviewboard.kde.org/r/5704/#comment8736>

    add a hint about what should be used instead.



/trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h
<http://svn.reviewboard.kde.org/r/5704/#comment8737>

    just as a sidenote: If in KDE5 you would make methods in this class virtual it would mean that one could implement custom calendarsystems.
    However, these would then not be reflected in the enum. Thus, they could not be managed in the same way, i.e. listed via calendarSystemList() and so on.



/trunk/KDE/kdelibs/kdecore/localization/klocale.h
<http://svn.reviewboard.kde.org/r/5704/#comment8738>

    How about a hint to the methods that use the enum?


- Sebastian


On 2010-10-28 12:56:04, John Layt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5704/
> -----------------------------------------------------------
> 
> (Updated 2010-10-28 12:56:04)
> 
> 
> 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/20101029/2606b484/attachment.htm>