Review Request: Add more date formatting options

John Layt johnlayt at googlemail.com
Sun Nov 22 16:33:08 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?
> 
> Albert Astals Cid wrote:
>     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
> 
> John Layt wrote:
>     OK, one more crack at a compromise.
>     
>     I tried adding a bunch more enums to DateFormat for all the variations people might want and I came up with about 20 or so I'd find useful, but the names are just so ugly and inflexible and it's bound to miss a format someone wants, it seemed an abuse of the purpose of that enum, and didn't really address the root of the problem that date order is usually locale based not language based.  It doesn't seem very useful to me, more like a straightjacket for coders and not very KDE.
>     
>     Next I tried going the Flags route.  This was a new enum DateFormatOption with 16 flag values for each of the possible components, e.g. YearLong, YearShort, WeekdayLong, WeekdayShort, etc.  Problem with this is the there would have been about 2430 valid combinations (and that's before seperators and order are considered), which is fine if generating it via code, but I would have had to type each one out for translation, which is just too many.  Besides, I couldn't see why a coder would find it easier to type "YearLong && MonthNameLong && DayLong  && SeparatorSpaces && IsoOrder" rather than "%Y %B %d", even if they did get the i18n() for free.
>     
>     The final option I've tried is a mixture of extending the current DateFormat enum and making a flags DateFormatOptions similar to the TimeFormatOptions flags, while also adding a DateFormatType enum as well:
>     
>         /**
>          * Format for date string.
>          *
>          * Some DateFomats can be modifed using DateFormatOptions.  The locale
>          * formats and ISO standard formats are not modified by the options.
>          */
>         enum DateFormat {
>             ShortDate,        /**< Locale Short date format, e.g. 08-04-2007 */
>             LongDate,         /**< Locale Long date format, e.g. Sunday 08 April 2007 */
>             FancyShortDate,   /**< Same as ShortDate for dates a week or more ago. For more
>                                    recent dates, it is represented as Today, Yesterday, or
>                                    the weekday name. */
>             FancyLongDate,    /**< Same as LongDate for dates a week or more ago. For more
>                                    recent dates, it is represented as Today, Yesterday, or
>                                    the weekday name. */
>             IsoDate,          /**< ISO-8601 Date format YYYY-MM-DD, e.g. 2009-12-31 */
>             IsoWeekDate,      /**< ISO-8601 Week Date format YYYY-Www-D, e.g. 2009-W01-1 */
>             IsoOrdinalDate,   /**< ISO-8601 Ordinal Date format YYYY-DDD, e.g. 2009-001 */
>             SimpleDate,       /**< Date with short day and month but no year,
>                                    in the locale DateFormatType,
>                                    e.g. 31/1 or 1/31 */
>             AbbreviatedDate,  /**< Date with short day, month and year
>                                    in the locale DateFormatType,
>                                    e.g. 31/1/9 or 1/31/9 */
>             StandardDate,     /**< Date with long day, month and year
>                                    in the locale DateFormatType,
>                                    e.g. 31/01/2009 or 01/31/2009 */
>             TruncatedDate,    /**< Date with long month and year
>                                    in the locale DateFormatType,
>                                    e.g. 01/2009 */
>         };
>     
>         /**
>          * @since 4.4
>          *
>          * Options for modifying the DateFormat, not all formats and options are
>          * compatible.
>          */
>         enum DateFormatOption {
>             DateWithShortComponants = 0x01, /**< Format with numbers and names in Short format,
>                                                  e.g. 1 or Jan */
>             DateWithLongComponants  = 0x02, /**< Format with numbers and names in Long format,
>                                                  e.g. 01 or January */
>             DateWithMonthNames      = 0x04, /**< Format with Month Name instead of number */
>             DateWithWeekdayNames    = 0x08  /**< Format with Weekday Name prefixed */
>         };
>         Q_DECLARE_FLAGS(DateFormatOptions, DateFormatOption)
>     
>         /**
>          * @since 4.4
>          *
>          * Preferred format type for a date, in particular the order for Year,
>          * Months and Day, and the default separators used.  These defaults may be
>          * overriden by the translators.
>          */
>         enum DateFormatType {
>             InternationalFormat, /**< Order as Day Month Year
>                                       Separate Numeric format with slashes
>                                       Separate Name format with spaces */
>             AmericanFormat,      /**< Order as Month Day Year
>                                       Separate Numeric format with slashes
>                                       Separate Name format with spaces */
>             IsoFormat            /**< Order as Year Month Day
>                                       All components in long form
>                                       Separate Numeric format with dashes
>                                       Separate Name format with spaces */
>         };
>     
>     
>     DateFormat gets 4 new formats called Simple, Abbreviated, Standard, and Truncated.  These can be modified by the DateFormatOptions to have weekday names, month names, and short or long format components.  They can also be modified by the DateFormatType which primarially controls the order and separators, i.e. American or Normal order.  The DateFormatType will be set in KLocale where it will default to InternationalFormat, and be overridden in the us.desktop locale file to AmericanFormat.  This gives about 87 valid combinations to code and translate, a more manageable number while giving simplicity and flexibility for coders.
>     
>     New API calls are required to support this:
>     
>         QString formatDate( const QDate &fromDate,
>                             KLocale::DateFormat toFormat,
>                             KLocale::DateFormatOptions formatOptions ) const;
>     
>         QString formatDate( const QDate &fromDate,
>                             KLocale::DateFormat toFormat,
>                             KLocale::DateFormatOptions formatOptions,
>                             KLocale::DateFormatType formatType ) const;
>     
>     So most of the time the app coder will only have to write something like the following without having to worry about translation or order or anything:
>     
>         dateString = KGlobal::locale()->calendar()->formatDate(myDate, KLocale::SimpleDate);
>     
>     or to modify the format slightly:
>     
>         dateString = KGlobal::locale()->calendar()->formatDate(myDate,
>                                                                KLocale::SimpleDate,
>                                                                KLocale::DateWithWeekdayNames);
>     
>     of if they really need a particular order:
>     
>         dateString = KGlobal::locale()->calendar()->formatDate(myDate,
>                                                                KLocale::SimpleDate,
>                                                                KLocale::DateWithWeekdayNames,
>                                                                KLocale::AmericanFormat);
>     
>     I think that's more like what you would like to see?  I like that it is easier for the coder not having to worry about translations, and gives priority for date order to the locale over the language but still allows the language to override if needed.  However behind the scenes in formatDate() there would be a lot of stuff like:
>     
>         if ( toFormat == KLocale::SimpleDate ) {
>             if ( dateOptions & KLocale::DateWithWeekdayNames &&
>                  dateOptions & KLocale::DateWithMonthNames   &&
>                  dateOptions & KLocale::DateWithLongComponants ) {
>                 if ( locale()->dateFormatOrder() == KLocale::InternationalFormat ) {
>                     formatString = i18nc("(kdedt-format) SimpleDate  InternationalFormat", "%a %d %B");
>                 } else if ( locale()->dateFormatOrder() == KLocale::AmericanFormat ) {
>                     formatString = i18nc("(kdedt-format) SimpleDate AmericanFormat", "%a %B %d");
>                 } else if ( locale()->dateFormatOrder() == KLocale::IsoFormat ) {
>                     formatString = i18nc("(kdedt-format) SimpleDate IsoOrder", "%a %B %d");
>                 }
>             } else if ( dateOptions & KLocale::DateWithWeekdayNames &&
>             ...
>     
>     I haven't fully coded this yet, as I don't want to type 87 date strings and their unit tests if there's still changes needed.  Any suggestions for a better mixture of DateFormat and DateFormatOptions?
>     
>     One thought is if we add more DateFormatTypes to allow for each possible set of formatting rules, then we may not need to translate the format string at all as we would have all the info we need about separators and order and length to generate the right format strings in code to start with?  e.g. add ones like EuropeanFormat for dd.mm.yyyy, ChineseFormat, JapaneseFormat, and a LanguageFormat one that uses a translated version for when different languages in a locale do use different formats (e.g. India), etc.  That could be a lot less code and translations but would need a lot more planning.
>     
>     Still, it seems like a lot of work for something that won't be used that often?  I would still leave the full version public anyway for those very rare occasions where it really is needed (kdepim, date widgets), just with very big warnings and sample code to dissuade people from using it.
> 
> Chusslove Illich wrote:
>     I've dropped the idea about enums, or any other non-format string based combinations of date formats, precisely for the reasons you describe. Too much combinations, too much to guess what is "mostly" needed, heavy-handed on programmers, and confusion between locale and language scopes.
>     
>     Instead I'd leave it as it is. Enums only for ordinary/fancy long/short formats, and the general formatting function for any other need. I would only insist on the special keyword in context of such strings, so that translations can be automatically checked for validity. It's already mentioned in the API doc, so only if you could rephrase it such that the probability of programmer heeding it is higher :)
>
> 
> Albert Astals Cid wrote:
>     Ok, if both Chusslove and John think enums are too much work for what it gives, let's just ditch them, the one who does the work decides and for sure i'm not doing the work so i don't want to decide here :-)

Thanks guys, see commit 1052841.  I'll keep the DateFormatType at locale level idea in mind for 4.5, if I knew I had a full set of the required formatting rules and could code something that didn't need (much) translation, I'll have another crack then.


- 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
> 
>





More information about the kde-core-devel mailing list