[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Add more date formatting options
From:       "John Layt" <johnlayt () googlemail ! com>
Date:       2009-11-22 16:33:08
Message-ID: 20091122163308.26799.86498 () localhost
[Download RAW message or body]



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


[prev in list] [next in list] [prev in thread] [next in thread]