The existing toString/toSymbolString methods use KLocalizedString::subs(),
which itself uses QString::arg() for creating the number. Which does
not take any QLocale into account.
The new overload method with a QLocale argument allow the caller full
control over which locale is used. Sadly QLocale does not have a
toString() method taking a fieldWidth parameter, so the overloads cannot
offer that feature.
Details
- Reviewers
ilic
Unit tests still pass (need english system locale though, needs separate
fix)
Diff Detail
- Repository
- R292 KUnitConversion
- Branch
- addToStringWithLocale
- Lint
No Linters Available - Unit
No Unit Test Coverage
As discussed on irc with @broulik will give that other patch https://git.reviewboard.kde.org/r/127800/ some look again, as I would agree numbers rather should be localized there.
Had considered something like that before as well, but then discarded due to not sure if changing behaviour at age of kf 5.46 is good and also as https://doc.qt.io/qt-5/qstring.html#arg-8 hinted that without QLocale::setDefault() being called, which the average app seems not to do, still C locale would be used. Which might not be the complete truth in the docs, will give some testing tonight.
The docu of ki18n says we localize numbers (as far as i remember), so that patch you discarded is "the right thing" imho
Sadly lost track of things, and no plans to look soon again at it, so cleaning up from stack for now.