Review Request: Add more date formatting options
Albert Astals Cid
aacid at kde.org
Tue Nov 17 23:31:44 GMT 2009
> 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.
>
> John Layt wrote:
> P.S. I think I would be more open to readDate() being private only, it's more open to error.
>
> Albert Astals Cid wrote:
> I agree the language is not the perfect heuristic to get the correct ordering but better than always showing it wrong, right?
>
> In my opinion enums would be the best solution, Chusslove had patch that did that exactly but don't remember why he stopped pushing for it.
>
> John Layt wrote:
> OK, I'll make them protected for 4.4 and see what I can do about some enums before the freeze, but that's more likely to come in 4.5. Hmm, that changes my patch for the Plasma clock, I'll have to go back to the old way.
>
> Actually, I can't make the readDate() protected, it's been public in both KLocale and KCalendarSystem since 4.0, it's only formatDate() that I introduced as new in 4.4.
>
> John Layt wrote:
> More thoughts after looking at how more apps use date formatting. I'm now inclined to make this public in 4.4, as I think we are making our app developers jump through hoops for no good reason, and they're making mistakes in the process. Where apps do need one-off date formats they should be able to generate them as required in a controlled and standardised way rather than resorting to hacks, workarounds, or their own implementations that may not localise the calendar system or digit set properly.
>
> In addition to people doing the copy locale trick described above, and a couple of places where apps temporarily change the global locale format (!), there are a lot of places using the string functions to do things like:
>
> *header = i18nc("Month String - Year String", "%1 %2",
> KGlobal::locale()->calendar()->monthName(album->date(), KCalendarSystem::LongName),
> KGlobal::locale()->calendar()->yearString(album->date()));
>
> And even more complex translation puzzles with 4 or 5 variables and literal characters.
>
> So people are in effect doing their own date format strings just the long way around, and almost all are being translated already. I don't see much wrong with giving them an easier way to do it that will also be easier and more consistent for the translators to follow and convert to their locally preferred format including literals, e.g. for the above (and the context is almost un-needed now):
>
> *header = KGlobal::locale()->calendar()->formatDate( album->date()
> i18nc("Month String - Year String", "%B %Y") );
>
> The kdepim guys have even implemented a whole class in KMime::DateFormatter to do date formatting for them using a mixture of named formats in an enum and custom formatting using QDate and even calling strftime() directly (so not KDE localised which is correct for RFC formats, but not if used for any display formats where they could get the wrong calendar system or digit set).
>
> A couple of plasmoids have also fully re-implemented the formatting standard as a wrapper around KCalendarSystem for their own purposes.
>
> Thinking about using enums instead, I've been thinking what ones would be needed for the Plasma Clock, which is configurable to optionally display various date portions, so would need at least the following enums:
>
> DayShortNumber_MonthShortName
> DayShortNumber_MonthShortName_YearNumber
> WeekdayShortName_DayShortNumber_MonthShortName
> WeekdayShortName_DayShortNumber_MonthShortName_YearLongNumber
> WeekdayName
>
> And that's just the short names, other apps will need long name versions, some might want literal chars included, and the list would just get longer and longer as each app adds what they want even if no-one else needs them. Then there's problems around some of those formats not being valid for use with readDate(), so separate enums would be required for read and display, so the numbers double again... Really, it would just end up as a long list of 'approved' date format strings, just with long and unwieldy names people won't want to use, even if they are consistently translated across all of KDE..
>
> No, I think the global enum should be restricted to recognised and unambiguous standard formats like IsoDate, IsoWeekDate, IsoOrdinalDate, and the various RFC formats.
>
> To encourage consistency, we could also point people in the apidox to a TechBase page of standard i18nc() calls with standardised contexts and format strings?
Ok, if i can't convince you about the enums i don't have much more to say, other than good luck trying to fix the mess :D
- Albert
-----------------------------------------------------------
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
>
>
More information about the kde-core-devel
mailing list