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
Details
- Reviewers
ilic
Unit tests still pass (need english system locale though, needs separate
fix)
Diff Detail
Diff Detail
- Repository
- R292 KUnitConversion
- Branch
- addToStringWithLocale
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
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.
Comment Actions
The docu of ki18n says we localize numbers (as far as i remember), so that patch you discarded is "the right thing" imho
Comment Actions
Sadly lost track of things, and no plans to look soon again at it, so cleaning up from stack for now.