D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2018-07-11 Thread Karl Ove Hufthammer 
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.

REPOSITORY
  R292 KUnitConversion

REVISION DETAIL
  https://phabricator.kde.org/D12164

To: kossebau, ilic
Cc: kde-frameworks-devel, huftis, broulik, michaelh, ngraham, bruns, #frameworks

D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2018-07-16 Thread Albert Astals Cid 
aacid added a comment.


  The docu of ki18n says we localize numbers (as far as i remember), so that 
patch you discarded is "the right thing" imho

REPOSITORY
  R292 KUnitConversion

REVISION DETAIL
  https://phabricator.kde.org/D12164

To: kossebau, ilic
Cc: aacid, kde-frameworks-devel, huftis, broulik, michaelh, ngraham, bruns

D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2018-04-12 Thread Friedrich W . H . Kossebau 
kossebau created this revision.
kossebau added a reviewer: ilic.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
kossebau requested review of this revision.

REVISION SUMMARY
  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.

TEST PLAN
  Unit tests still pass (need english system locale though, needs separate
  fix)

REPOSITORY
  R292 KUnitConversion

BRANCH
  addToStringWithLocale

REVISION DETAIL
  https://phabricator.kde.org/D12164

AFFECTED FILES
  autotests/valuetest.cpp
  src/unit.cpp
  src/unit.h
  src/value.cpp
  src/value.h

To: kossebau, ilic
Cc: #frameworks, michaelh, ngraham, bruns

D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2018-04-13 Thread Kai Uwe Broulik 
broulik added a comment.


  +1 always annoyed me that the converter runner didn't use localed decimal 
points
  
  See also https://git.reviewboard.kde.org/r/127800/

REPOSITORY
  R292 KUnitConversion

REVISION DETAIL
  https://phabricator.kde.org/D12164

To: kossebau, ilic
Cc: broulik, #frameworks, michaelh, ngraham, bruns

D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2018-04-13 Thread Friedrich W . H . Kossebau 
kossebau added a comment.


  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.

REPOSITORY
  R292 KUnitConversion

REVISION DETAIL
  https://phabricator.kde.org/D12164

To: kossebau, ilic
Cc: broulik, #frameworks, michaelh, ngraham, bruns

D12164: Add overloads to Value/Unit::toString/toSymbolString taking a QLocale

2019-11-29 Thread Friedrich W. H. Kossebau 
kossebau abandoned this revision.
kossebau added a comment.


  Sadly lost track of things, and no plans to look soon again at it, so 
cleaning up from stack for now.

REPOSITORY
  R292 KUnitConversion

REVISION DETAIL
  https://phabricator.kde.org/D12164

To: kossebau, ilic
Cc: aacid, kde-frameworks-devel, huftis, broulik, LeGast00n, GB_2, michaelh, 
ngraham, bruns