Review Request: Rewrite of KLocale::readTime() and KLocale::formatTime()

[Michael Leupold]

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

(Updated 2009-07-20 22:03:02.824793)


Review request for kdelibs.


Changes
-------

Changed readTime() after discovering a bug in my code: lax processing hast to skip at most one space as the input string is already simplified().

I'm wondering if we really need strict and lax processing or if it might be better to just allow what's currently "lax" processing (only difference right now is that strict processing requires spaces, lax processing doesn't).


Summary
-------

While working on a QValidator for time entry I found KLocale::readTime() and KLocale::formatTime() to be somewhat lacking in certain areas:
- stripping of AM/PM ("%p") doesn't work for all locales
- if I don't want AM/PM I need to format as a duration (which means it will be 24h) which is inconvenient in cases where the AM/PM is "external" (eg. inside a combobox to choose from). Not having to mod 12 the hours beforehand seems more convenient.
- checking is always strict. As the method reads user input there should be a way to make it operate in lax mode (ie. ignoreing missing or additional spaces in user input)
- readTime() has less features than formatTime (no am/pm stripping or durations).
- signatures for readTime() and formatTime() could be adapted to match each other somewhat.

I tried to achieve all of the above with a partial rewrite of those methods using the old code and some code I wrote while developing the validator. I also wrote an additional testcase for reading/formatting times.


Diffs (updated)
-----

  /trunk/KDE/kdelibs/kdecore/localization/klocale.h 1000119 
  /trunk/KDE/kdelibs/kdecore/localization/klocale.cpp 1000119 
  /trunk/KDE/kdelibs/kdecore/tests/CMakeLists.txt 1000119 
  /trunk/KDE/kdelibs/kdecore/tests/klocaletimeformattest.h PRE-CREATION 
  /trunk/KDE/kdelibs/kdecore/tests/klocaletimeformattest.cpp PRE-CREATION 

Diff: http://reviewboard.kde.org/r/1065/diff


Testing
-------

klocaletest succeeds, the new testcase succeeds as well.


Thanks,

Michael