Review Request: Add more date formatting options

[John Layt]

> On 2009-11-15 15:14:38, Albert Astals Cid wrote:
> > In my opinion, this has a problem and it is that it encourages developers to do things like
> > 
> > QString myDateString = KGlobal::locale()->calendar()->dateFormat( myDate, "%m %d" );
> > 
> > Which has internationalization problems since %m %d is obviously the wrong format for at least half of the world that uses "%d %m" so i'd like to ask if we really need that much flexibility and if we do i'd ask you to empathize in documentation that the code should be something like
> > 
> > QString myDateString = KGlobal::locale()->calendar()->dateFormat( myDate, i18nc("This is a string that defines how date is shown when doing foo in bar program check with someurl what is the valid syntax", "%m %d") );
> 
> John Layt wrote:
>     There is doco already in the apidox saying to i18nc() the format string and giving example code, but I'll move it up to the top to make it more prominent.
>     
>     There's already plenty of places where people do already do their own format strings by copying the global KLocale and calling setDateFormat(), so it is something that apps do need, and it's better we provide them with a way of doing it that makes it easier for them to get it right, rather then leave it open for them to make a mistake and possibly mess up the global format.  Also language is not always a good heuristic for date formats, take for example someone living in the USA but using Spanish language, which way round should it go then? (which I realise is as good an argument against this change being public as for it).
>     
>     Putting the method in KCalendarSystem rather than KLocale makes it harder to find but still there for those who really need it.
>     
>     If people feel it will be abused, then I can make it private so the only formats available will be the predefined enum KLocale::DateFormat which currently are the ShortDate and LongDate forms set in l10n/KCM.  I can then add to the enum some other standard formats like IsoWeekDate and DayMonthOnly that can be passed into formatDate() and readDate(), although this needs more thought and may require separate input and output format enums (all this is already in the 4.4 feature plan but may be more realistic for 4.5).  How user maintainable those would be is open to debate, users would need to be able to set the month/day order on some of them at least.

P.S. I think I would be more open to readDate() being private only, it's more open to error.


- John


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


On 2009-11-15 02:21:15, John Layt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2177/
> -----------------------------------------------------------
> 
> (Updated 2009-11-15 02:21:15)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This change adds a number of new options for date formatting.
> 
> 1) New versions of formatDate() and readDate() have already been added to KCalendarSystem in 4.4 to accept a date format string of the coders choice rather than the locale defaults.  This saves coders writing something like:
> 
>     KLocale *myLocale = new KLocale( *KGlobal::locale() );
>     myLocale->setDateFormatShort( "%m %d" );
>     QString myDateString = myLocal->dateFormat( myDate, KLocale::ShortDate );
> 
> They can now write:
> 
>     QString myDateString = KGlobal::locale()->calendar()->dateFormat( myDate, "%m %d" );
> 
> This change adds new format codes to these methods to bring them into line with the C / POSIX / CU / GNU standards:
> 
> The following codes are added to formatDate():
> 
>     %C the 'century' portion of the year
>     %j the day of the year number to three digits (e.g. "001"  for 1 Jan)
>     %V the ISO week of the year number to two digits
>     %G the year number in long form of the ISO week of the year
>     %g the year number in short form of the ISO week of the year
>     %u the day of the week number (e.g. "1"  for Monday)
>     %D the US short date format (e.g. "%m/%d/%y")
>     %F the ISO short date format (e.g. "%Y-%m-%d")
>     %x the KDE locale short date format
>     %t a tab character
> 
> formatDate() also adds the ability to define the case modifier, padding character, and padding width as defined in the GNU version of the standard, e.g. "%05Y" = "02009".  See the comprehensive apidox documentation for details.
> 
> There are 2 significant differences between the long-standing KDE implementation and the standard:
> 
>     %e in GNU/POSIX is space padded to 2 digits, in KDE is not padded
>     %n in GNU/POSIX is newline, in KDE is short month number
> 
> Should we fix these to match the standard, or just live with them?
> 
> The following codes are added to readDate():
> 
> 	%j the day of the year number
> 	%V the ISO week of the year number
> 	%u the day of the week number
> 
> Note that only the codes supported by readDate() will be allowed to be used in the locale global date formats, and coders are encouraged to use the locale formats unless really necessary.
> 
> 2) New isValid() and setDate() methods to validate and set the date using either Year and Day of Year, or Year, ISO Week Number, and Day of Week number, as required by the readDate() changes above and as defined in the ISO standard.
> 
> 3) KDE4.3 introduced the ability to use different numeric Digit Sets, e.g. Arabic-Indic or Devenagari instead of Arabic.  While some numeric methods in KCalendarSystem have versions to return a string form which returns the correct digit set (e.g. day() and dayString()), many do not have a string version such as weekNumber().  This means a coder needs to know and remember to do the digit set conversion themselves:
> 
>     KGlobal::locale()->convertDigits(KGlobal::locale()->calendar()->weekNumber(date), locale()->dateTimeDigitSet());
> 
> This change adds the following convenience methods to improve this:
> 
>     monthsInYearString()
>     weeksInYearString()
>     daysInYearString()
>     daysInMonthString()
>     daysInWeekString()
>     dayOfYearString()
>     dayOfWeekString()
>     weekNumberString()
> 
> The existing string methods have all been converted to use the formatDate() method.
> 
> 4) A change this big to something so core couldn't be done without significant testing.  The first real comprehensive unit tests for formatDate() and readDate() are included.  In fact, the test cases took longer to write than the code :-)
> 
> 5) A number of corner-case bugs were uncovered in the existing code by the new test cases which have been fixed as well.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.h 1046844 
>   trunk/KDE/kdelibs/kdecore/date/kcalendarsystem.cpp 1046844 
>   trunk/KDE/kdelibs/kdecore/date/kcalendarsystemgregorian.cpp 1046844 
>   trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h 1046844 
>   trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp 1046844 
> 
> Diff: http://reviewboard.kde.org/r/2177/diff
> 
> 
> Testing
> -------
> 
> Extensive new unit tests written, as attached, which showed up a number of bugs in the existing code.  More tests to be added for rest of calendar systems in next few weeks.
> 
> 
> Thanks,
> 
> John
> 
>