[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:       "Albert Astals Cid" <aacid () kde ! org>
Date:       2009-11-15 21:24:57
Message-ID: 20091115212457.1754.30610 () 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.

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.


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


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