From 3e44db5a04e5f0134c557ac893c79253a850281d Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 5 Jun 2018 16:33:47 +0200 Subject: [PATCH 01/13] Use QLocale when representing double fields --- src/core/qgsfield.cpp | 27 +++++++++++++++++++++++++-- tests/src/core/testqgsfield.cpp | 15 ++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp index a9d2a18fc04c..6886625d8829 100644 --- a/src/core/qgsfield.cpp +++ b/src/core/qgsfield.cpp @@ -22,6 +22,7 @@ #include #include +#include /*************************************************************************** * This class is considered CRITICAL and any change MUST be accompanied with @@ -208,8 +209,30 @@ QString QgsField::displayString( const QVariant &v ) const return QgsApplication::nullRepresentation(); } - if ( d->type == QVariant::Double && d->precision > 0 ) - return QString::number( v.toDouble(), 'f', d->precision ); + if ( d->type == QVariant::Double ) + { + if ( d->precision > 0 ) + { + return QLocale().toString( v.toDouble(), 'f', d->precision ); + } + else + { + // Precision is not set, let's guess it from the + // standard conversion to string + QString s( v.toString() ); + int dotPosition( s.indexOf( '.' ) ); + int precision; + if ( dotPosition < 0 ) + { + precision = 0; + } + else + { + precision = s.length() - dotPosition - 1; + } + return QLocale().toString( v.toDouble(), 'f', precision ); + } + } return v.toString(); } diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp index 5a7081260553..35876147af36 100644 --- a/tests/src/core/testqgsfield.cpp +++ b/tests/src/core/testqgsfield.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -66,12 +67,12 @@ void TestQgsField::cleanupTestCase() void TestQgsField::init() { - + QLocale::setDefault( QLocale::English ); } void TestQgsField::cleanup() { - + QLocale::setDefault( QLocale::English ); } void TestQgsField::create() @@ -332,12 +333,20 @@ void TestQgsField::displayString() QgsField doubleFieldNoPrec( QStringLiteral( "double" ), QVariant::Double, QStringLiteral( "double" ), 10 ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5.005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5.005005005" ) ); - QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599999898999" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599,999,898,999" ) ); //test NULL double QVariant nullDouble = QVariant( QVariant::Double ); QCOMPARE( doubleField.displayString( nullDouble ), QString( "TEST NULL" ) ); + //test double value with German locale + QLocale::setDefault( QLocale::German ); + QCOMPARE( doubleField.displayString( 5.005005 ), QString( "5,005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5,005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5,005005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599.999.898.999" ) ); + + } void TestQgsField::convertCompatible() From e56b72afbb5404c5dc059416e1fd549bf443bee1 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 5 Jun 2018 18:14:05 +0200 Subject: [PATCH 02/13] Use displayString to get the formatted value --- src/gui/editorwidgets/qgstexteditwrapper.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gui/editorwidgets/qgstexteditwrapper.cpp b/src/gui/editorwidgets/qgstexteditwrapper.cpp index 989f6ac31929..25fd2245e62d 100644 --- a/src/gui/editorwidgets/qgstexteditwrapper.cpp +++ b/src/gui/editorwidgets/qgstexteditwrapper.cpp @@ -217,7 +217,9 @@ void QgsTextEditWrapper::setWidgetValue( const QVariant &val ) v = QgsApplication::nullRepresentation(); } else - v = val.toString(); + { + v = field().displayString( val ); + } if ( mTextEdit ) { From 26280751a6395e1eee71e51ba36f7168b419e191 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 5 Jun 2018 18:15:27 +0200 Subject: [PATCH 03/13] Try to convert doubles with comma as decimal point --- src/core/qgsfield.cpp | 7 +++++++ tests/src/core/testqgsfield.cpp | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp index 6886625d8829..e34b926cf4bc 100644 --- a/src/core/qgsfield.cpp +++ b/src/core/qgsfield.cpp @@ -281,6 +281,13 @@ bool QgsField::convertCompatible( QVariant &v ) const return true; } + // Give it a chance to convert to double since we accept both comma and dot as decimal point + QVariant tmp( v ); + if ( d->type == QVariant::Double && !tmp.convert( d->type ) ) + { + v = v.toString().replace( ',', '.' ); + } + if ( !v.convert( d->type ) ) { v = QVariant( d->type ); diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp index 35876147af36..8c95b6ea3320 100644 --- a/tests/src/core/testqgsfield.cpp +++ b/tests/src/core/testqgsfield.cpp @@ -346,7 +346,6 @@ void TestQgsField::displayString() QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5,005005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599.999.898.999" ) ); - } void TestQgsField::convertCompatible() @@ -475,6 +474,12 @@ void TestQgsField::convertCompatible() QVERIFY( !stringWithLen.convertCompatible( stringVar ) ); QCOMPARE( stringVar.type(), QVariant::String ); QCOMPARE( stringVar.toString(), QString( "lon" ) ); + + //double with ',' as decimal separator + QVariant doubleCommaVar( "1,2345" ); + QVERIFY( doubleField.convertCompatible( doubleCommaVar ) ); + QCOMPARE( doubleCommaVar.type(), QVariant::Double ); + QCOMPARE( doubleCommaVar.toString(), QString( "1.2345" ) ); } void TestQgsField::dataStream() From ec20ff78d1fa265e3b83034d49797769ef731ef0 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 5 Jun 2018 18:16:59 +0200 Subject: [PATCH 04/13] Accept dot and comma as decimal point on input --- src/gui/qgsfieldvalidator.cpp | 10 +-- tests/src/python/test_qgsfieldvalidator.py | 76 +++++++++++++++------- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/gui/qgsfieldvalidator.cpp b/src/gui/qgsfieldvalidator.cpp index bcc3ebe0e568..2d7905342dd8 100644 --- a/src/gui/qgsfieldvalidator.cpp +++ b/src/gui/qgsfieldvalidator.cpp @@ -56,7 +56,7 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co { if ( mField.length() > 0 && mField.precision() > 0 ) { - QString re = QStringLiteral( "-?\\d{0,%1}(\\.\\d{0,%2})?" ).arg( mField.length() - mField.precision() ).arg( mField.precision() ); + QString re = QStringLiteral( "-?\\d{0,%1}([\\.,]\\d{0,%2})?" ).arg( mField.length() - mField.precision() ).arg( mField.precision() ); mValidator = new QRegExpValidator( QRegExp( re ), parent ); } else if ( mField.length() > 0 && mField.precision() == 0 ) @@ -66,7 +66,7 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co } else if ( mField.precision() > 0 ) { - QString re = QStringLiteral( "-?\\d*(\\.\\d{0,%1})?" ).arg( mField.precision() ); + QString re = QStringLiteral( "-?\\d*([\\.,]\\d{0,%1})?" ).arg( mField.precision() ); mValidator = new QRegExpValidator( QRegExp( re ), parent ); } else @@ -112,12 +112,6 @@ QValidator::State QgsFieldValidator::validate( QString &s, int &i ) const // delegate to the child validator if any if ( mValidator ) { - // force to use the '.' as a decimal point or in case we are using QDoubleValidator - // we can get a valid number with a comma depending on current locale - // ... but this will fail subsequently when converted from string to double and - // becomes a NULL! - if ( mField.type() == QVariant::Double ) - s = s.replace( ',', '.' ); QValidator::State result = mValidator->validate( s, i ); return result; } diff --git a/tests/src/python/test_qgsfieldvalidator.py b/tests/src/python/test_qgsfieldvalidator.py index 607277194d8f..e1e4c36190d3 100644 --- a/tests/src/python/test_qgsfieldvalidator.py +++ b/tests/src/python/test_qgsfieldvalidator.py @@ -14,7 +14,7 @@ import qgis # NOQA -from qgis.PyQt.QtCore import QVariant +from qgis.PyQt.QtCore import QVariant, QLocale from qgis.PyQt.QtGui import QValidator from qgis.core import QgsVectorLayer from qgis.gui import QgsFieldValidator @@ -39,45 +39,41 @@ def tearDown(self): """Run after each test.""" pass - def test_validator(self): - # Test the double - """ + def _fld_checker(self, field): + """ Expected results from validate QValidator::Invalid 0 The string is clearly invalid. QValidator::Intermediate 1 The string is a plausible intermediate value. QValidator::Acceptable 2 The string is acceptable as a final result; i.e. it is valid. """ + DECIMAL_SEPARATOR = QLocale().decimalPoint() + OTHER_SEPARATOR = ',' if DECIMAL_SEPARATOR == '.' else '.' - double_field = self.vl.fields()[self.vl.fields().indexFromName('double_field')] - self.assertEqual(double_field.precision(), 0) # this is what the provider reports :( - self.assertEqual(double_field.length(), 0) # not set - self.assertEqual(double_field.type(), QVariant.Double) - - validator = QgsFieldValidator(None, double_field, '0.0', '') + validator = QgsFieldValidator(None, field, '0.0', '') def _test(value, expected): ret = validator.validate(value, 0) - self.assertEqual(ret[0], expected, value) + self.assertEqual(ret[0], expected, "%s != %s" % (ret[0], expected)) if value: self.assertEqual(validator.validate('-' + value, 0)[0], expected, '-' + value) - # Check the decimal comma separator has been properly transformed - if expected != QValidator.Invalid: - self.assertEqual(ret[1], value.replace(',', '.')) # Valid _test('0.1234', QValidator.Acceptable) _test('0,1234', QValidator.Acceptable) - _test('12345.1234e+123', QValidator.Acceptable) - _test('12345.1234e-123', QValidator.Acceptable) - _test('12345,1234e+123', QValidator.Acceptable) - _test('12345,1234e-123', QValidator.Acceptable) - _test('', QValidator.Acceptable) - # Out of range - _test('12345.1234e+823', QValidator.Intermediate) - _test('12345.1234e-823', QValidator.Intermediate) - _test('12345,1234e+823', QValidator.Intermediate) - _test('12345,1234e-823', QValidator.Intermediate) + # If precision is > 0, regexp validator is used (and it does not support sci notation) + if field.precision() == 0: + _test('12345.1234e+123', QValidator.Acceptable) + _test('12345.1234e-123', QValidator.Acceptable) + _test('12345,1234e+123', QValidator.Acceptable) + _test('12345,1234e-123', QValidator.Acceptable) + _test('', QValidator.Acceptable) + + # Out of range + _test('12345.1234e+823', QValidator.Intermediate) + _test('12345.1234e-823', QValidator.Intermediate) + _test('12345,1234e+823', QValidator.Intermediate) + _test('12345,1234e-823', QValidator.Intermediate) # Invalid _test('12345-1234', QValidator.Invalid) @@ -97,9 +93,39 @@ def _test(value, expected): # Invalid _test('12345-1234', QValidator.Invalid) - _test('12345.1234', QValidator.Invalid) + _test('12345%s1234' % DECIMAL_SEPARATOR, QValidator.Invalid) _test('onetwothree', QValidator.Invalid) + def test_doubleValidator(self): + """Test the double with default (system) locale""" + field = self.vl.fields()[self.vl.fields().indexFromName('double_field')] + self.assertEqual(field.precision(), 0) # this is what the provider reports :( + self.assertEqual(field.length(), 0) # not set + self.assertEqual(field.type(), QVariant.Double) + self._fld_checker(field) + + def test_doubleValidatorCommaLocale(self): + """Test the double with german locale""" + QLocale.setDefault(QLocale(QLocale.German, QLocale.Germany)) + assert QLocale().decimalPoint() == ',' + field = self.vl.fields()[self.vl.fields().indexFromName('double_field')] + self._fld_checker(field) + + def test_doubleValidatorDotLocale(self): + """Test the double with english locale""" + QLocale.setDefault(QLocale(QLocale.English)) + assert QLocale().decimalPoint() == '.' + field = self.vl.fields()[self.vl.fields().indexFromName('double_field')] + self._fld_checker(field) + + def test_precision(self): + """Test different precision""" + QLocale.setDefault(QLocale(QLocale.English)) + assert QLocale().decimalPoint() == '.' + field = self.vl.fields()[self.vl.fields().indexFromName('double_field')] + field.setPrecision(4) + self._fld_checker(field) + if __name__ == '__main__': unittest.main() From e7df793d6ac0de7a0f4e5da5fc40854e7c33ce0f Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 6 Jun 2018 13:39:56 +0200 Subject: [PATCH 05/13] Set locale in the tests --- tests/src/python/test_qgsfieldformatters.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/src/python/test_qgsfieldformatters.py b/tests/src/python/test_qgsfieldformatters.py index 6e7778cba5a0..f86ef1ad703d 100644 --- a/tests/src/python/test_qgsfieldformatters.py +++ b/tests/src/python/test_qgsfieldformatters.py @@ -266,8 +266,14 @@ def setUpClass(cls): QCoreApplication.setOrganizationDomain("QGIS_TestPyQgsColorScheme.com") QCoreApplication.setApplicationName("QGIS_TestPyQgsColorScheme") QgsSettings().clear() + QLocale.setDefault(QLocale.c()) start_app() + @classmethod + def tearDownClass(cls): + """Reset locale""" + QLocale.setDefault(QLocale.c()) + def test_representValue(self): layer = QgsVectorLayer("point?field=int:integer&field=double:double&field=long:long", @@ -277,8 +283,6 @@ def test_representValue(self): fieldFormatter = QgsRangeFieldFormatter() - QLocale.setDefault(QLocale.c()) - # Precision is ignored for integers and longlongs self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '123'), '123') self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '123000'), '123000') From 8c74ddebfa78b3b150e3fd433599e0c20f819073 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 16:21:32 +0200 Subject: [PATCH 06/13] Added tests for locales other than english --- tests/src/python/test_qgsfieldformatters.py | 28 +++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/src/python/test_qgsfieldformatters.py b/tests/src/python/test_qgsfieldformatters.py index f86ef1ad703d..7bee27288164 100644 --- a/tests/src/python/test_qgsfieldformatters.py +++ b/tests/src/python/test_qgsfieldformatters.py @@ -266,13 +266,13 @@ def setUpClass(cls): QCoreApplication.setOrganizationDomain("QGIS_TestPyQgsColorScheme.com") QCoreApplication.setApplicationName("QGIS_TestPyQgsColorScheme") QgsSettings().clear() - QLocale.setDefault(QLocale.c()) + QLocale.setDefault(QLocale(QLocale.English)) start_app() @classmethod def tearDownClass(cls): """Reset locale""" - QLocale.setDefault(QLocale.c()) + QLocale.setDefault(QLocale(QLocale.English)) def test_representValue(self): @@ -285,19 +285,19 @@ def test_representValue(self): # Precision is ignored for integers and longlongs self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '123'), '123') - self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '123000'), '123000') - self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '9999999'), '9999999') # no scientific notation for integers! + self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '123000'), '123,000') + self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '9999999'), '9,999,999') # no scientific notation for integers! self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, None), 'NULL') self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '123'), '123') - self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '123000'), '123000') - self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '9999999'), '9999999') # no scientific notation for long longs! + self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '123000'), '123,000') + self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '9999999'), '9,999,999') # no scientific notation for long longs! self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, None), 'NULL') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 1}, None, None), 'NULL') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 1}, None, '123'), '123.0') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, None), 'NULL') - self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '123000'), '123000.00') + self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '123000'), '123,000.00') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '0'), '0.00') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '123'), '123.00') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '0.123'), '0.12') @@ -312,6 +312,7 @@ def test_representValue(self): self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 3}, None, '-0.127'), '-0.127') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 3}, None, '-1.27e-1'), '-0.127') + # Check with Italian locale QLocale.setDefault(QLocale('it')) self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '9999999'), @@ -337,6 +338,19 @@ def test_representValue(self): self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 3}, None, '-0.127'), '-0,127') self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 3}, None, '-1.27e-1'), '-0,127') + # Check with custom locale without thousand separator + + custom = QLocale('en') + custom.setNumberOptions(QLocale.OmitGroupSeparator) + QLocale.setDefault(custom) + + self.assertEqual(fieldFormatter.representValue(layer, 0, {'Precision': 1}, None, '9999999'), + '9999999') # scientific notation for integers! + self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '123'), '123') + self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '123000'), '123000') + self.assertEqual(fieldFormatter.representValue(layer, 2, {'Precision': 1}, None, '9999999'), '9999999') # scientific notation for long longs! + self.assertEqual(fieldFormatter.representValue(layer, 1, {'Precision': 2}, None, '123000'), '123000.00') + QgsProject.instance().removeAllMapLayers() From f853c8d1d9e64131096dac16faaefb72cfeb976a Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 16:22:14 +0200 Subject: [PATCH 07/13] Respect locale decimal separator in input fields --- src/gui/qgsfieldvalidator.cpp | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/gui/qgsfieldvalidator.cpp b/src/gui/qgsfieldvalidator.cpp index 2d7905342dd8..5ae4a432ffcd 100644 --- a/src/gui/qgsfieldvalidator.cpp +++ b/src/gui/qgsfieldvalidator.cpp @@ -56,7 +56,16 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co { if ( mField.length() > 0 && mField.precision() > 0 ) { - QString re = QStringLiteral( "-?\\d{0,%1}([\\.,]\\d{0,%2})?" ).arg( mField.length() - mField.precision() ).arg( mField.precision() ); + QString re; + // Also accept locale's decimalPoint if it's not a dot + if ( QLocale().decimalPoint() != '.' ) + { + re = QStringLiteral( "-?\\d{0,%1}([\\.%2]\\d{0,%3})?" ).arg( mField.length() - mField.precision() ).arg( QLocale().decimalPoint() ).arg( mField.precision() ); + } + else + { + re = QStringLiteral( "-?\\d{0,%1}([\\.,]\\d{0,%2})?" ).arg( mField.length() - mField.precision() ).arg( mField.precision() ); + } mValidator = new QRegExpValidator( QRegExp( re ), parent ); } else if ( mField.length() > 0 && mField.precision() == 0 ) @@ -66,7 +75,16 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co } else if ( mField.precision() > 0 ) { - QString re = QStringLiteral( "-?\\d*([\\.,]\\d{0,%1})?" ).arg( mField.precision() ); + QString re; + // Also accept locale's decimalPoint if it's not a dot + if ( QLocale().decimalPoint() != '.' ) + { + re = QStringLiteral( "-?\\d*([\\.%1]\\d{0,%2})?" ).arg( QLocale().decimalPoint(), mField.precision() ); + } + else + { + re = QStringLiteral( "-?\\d*([\\.]\\d{0,%1})?" ).arg( mField.precision() ); + } mValidator = new QRegExpValidator( QRegExp( re ), parent ); } else From a85f67ee9152c835d117e558b8ee396c10ed0ade Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 16:23:27 +0200 Subject: [PATCH 08/13] Skip custom handling of decimal point if it's a dot --- src/core/qgsfield.cpp | 59 ++++++++++++++++++++++----------- tests/src/core/testqgsfield.cpp | 43 ++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 21 deletions(-) diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp index e34b926cf4bc..098908950562 100644 --- a/src/core/qgsfield.cpp +++ b/src/core/qgsfield.cpp @@ -209,31 +209,48 @@ QString QgsField::displayString( const QVariant &v ) const return QgsApplication::nullRepresentation(); } + // Special treatment for numeric types if group separator is set or decimalPoint is not a dot if ( d->type == QVariant::Double ) { - if ( d->precision > 0 ) + // Locales with decimal point != '.' or that require group separator: use QLocale + if ( QLocale().decimalPoint() != '.' || + !( QLocale().numberOptions() & QLocale::NumberOption::OmitGroupSeparator ) ) { - return QLocale().toString( v.toDouble(), 'f', d->precision ); - } - else - { - // Precision is not set, let's guess it from the - // standard conversion to string - QString s( v.toString() ); - int dotPosition( s.indexOf( '.' ) ); - int precision; - if ( dotPosition < 0 ) + if ( d->precision > 0 ) { - precision = 0; + return QLocale().toString( v.toDouble(), 'f', d->precision ); } else { - precision = s.length() - dotPosition - 1; + // Precision is not set, let's guess it from the + // standard conversion to string + QString s( v.toString() ); + int dotPosition( s.indexOf( '.' ) ); + int precision; + if ( dotPosition < 0 ) + { + precision = 0; + } + else + { + precision = s.length() - dotPosition - 1; + } + return QLocale().toString( v.toDouble(), 'f', precision ); } - return QLocale().toString( v.toDouble(), 'f', precision ); + } + // Default for doubles with precision + else if ( d->type == QVariant::Double && d->precision > 0 ) + { + return QString::number( v.toDouble(), 'f', d->precision ); } } - + // Other numeric types out of doubles + else if ( isNumeric() && + ! QLocale().numberOptions() & QLocale::NumberOption::OmitGroupSeparator ) + { + return QLocale().toString( v.toLongLong() ); + } + // Fallback if special rules do not apply return v.toString(); } @@ -281,11 +298,15 @@ bool QgsField::convertCompatible( QVariant &v ) const return true; } - // Give it a chance to convert to double since we accept both comma and dot as decimal point - QVariant tmp( v ); - if ( d->type == QVariant::Double && !tmp.convert( d->type ) ) + // Give it a chance to convert to double since for not '.' locales + // we accept both comma and dot as decimal point + if ( d->type == QVariant::Double && QLocale().decimalPoint() != '.' ) { - v = v.toString().replace( ',', '.' ); + QVariant tmp( v ); + if ( d->type == QVariant::Double && !tmp.convert( d->type ) ) + { + v = v.toString().replace( ',', '.' ); + } } if ( !v.convert( d->type ) ) diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp index 8c95b6ea3320..823a2cba6c09 100644 --- a/tests/src/core/testqgsfield.cpp +++ b/tests/src/core/testqgsfield.cpp @@ -316,12 +316,12 @@ void TestQgsField::displayString() //test int value in int type QgsField intField2( QStringLiteral( "int" ), QVariant::Int, QStringLiteral( "int" ) ); QCOMPARE( intField2.displayString( 5 ), QString( "5" ) ); - QCOMPARE( intField2.displayString( 599999898999LL ), QString( "599999898999" ) ); + QCOMPARE( intField2.displayString( 599999898999LL ), QString( "599,999,898,999" ) ); //test long type QgsField longField( QStringLiteral( "long" ), QVariant::LongLong, QStringLiteral( "longlong" ) ); QCOMPARE( longField.displayString( 5 ), QString( "5" ) ); - QCOMPARE( longField.displayString( 599999898999LL ), QString( "599999898999" ) ); + QCOMPARE( longField.displayString( 599999898999LL ), QString( "599,999,898,999" ) ); //test NULL int QVariant nullInt = QVariant( QVariant::Int ); @@ -333,6 +333,8 @@ void TestQgsField::displayString() QgsField doubleFieldNoPrec( QStringLiteral( "double" ), QVariant::Double, QStringLiteral( "double" ), 10 ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5.005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5.005005005" ) ); + QCOMPARE( QLocale().decimalPoint(), '.' ); + QCOMPARE( QLocale().numberOptions() & QLocale::NumberOption::OmitGroupSeparator, QLocale::NumberOption::DefaultNumberOptions ); QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599,999,898,999" ) ); //test NULL double @@ -345,6 +347,43 @@ void TestQgsField::displayString() QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5,005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5,005005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599.999.898.999" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5999.123456 ), QString( "5.999,123456" ) ); + + //test value with custom German locale (OmitGroupSeparator) + QLocale customGerman( QLocale::German ); + customGerman.setNumberOptions( QLocale::NumberOption::OmitGroupSeparator ); + QLocale::setDefault( customGerman ); + QCOMPARE( doubleField.displayString( 5.005005 ), QString( "5,005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5,005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5,005005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599999898999" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5999.123456 ), QString( "5999,123456" ) ); + + //test int value in int type with custom German locale (OmitGroupSeparator) + QCOMPARE( intField2.displayString( 5 ), QString( "5" ) ); + QCOMPARE( intField2.displayString( 599999898999LL ), QString( "599999898999" ) ); + + //test long type with custom German locale (OmitGroupSeparator) + QCOMPARE( longField.displayString( 5 ), QString( "5" ) ); + QCOMPARE( longField.displayString( 599999898999LL ), QString( "599999898999" ) ); + + //test value with custom english locale (OmitGroupSeparator) + QLocale customEnglish( QLocale::English ); + customEnglish.setNumberOptions( QLocale::NumberOption::OmitGroupSeparator ); + QLocale::setDefault( customEnglish ); + QCOMPARE( doubleField.displayString( 5.005005 ), QString( "5.005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5.005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5.005005005" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599999898999" ) ); + QCOMPARE( doubleFieldNoPrec.displayString( 5999.123456 ), QString( "5999.123456" ) ); + + //test int value in int type with custom english locale (OmitGroupSeparator) + QCOMPARE( intField2.displayString( 5 ), QString( "5" ) ); + QCOMPARE( intField2.displayString( 599999898999LL ), QString( "599999898999" ) ); + + //test long type with custom english locale (OmitGroupSeparator) + QCOMPARE( longField.displayString( 5 ), QString( "5" ) ); + QCOMPARE( longField.displayString( 599999898999LL ), QString( "599999898999" ) ); } From 85e34f2c929deface8fbc8b2edc0c9aae8d37802 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 17:13:14 +0200 Subject: [PATCH 09/13] Fix test comparison --- tests/src/core/testqgsfield.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp index 823a2cba6c09..a3ffc02abdd5 100644 --- a/tests/src/core/testqgsfield.cpp +++ b/tests/src/core/testqgsfield.cpp @@ -333,7 +333,6 @@ void TestQgsField::displayString() QgsField doubleFieldNoPrec( QStringLiteral( "double" ), QVariant::Double, QStringLiteral( "double" ), 10 ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005 ), QString( "5.005005" ) ); QCOMPARE( doubleFieldNoPrec.displayString( 5.005005005 ), QString( "5.005005005" ) ); - QCOMPARE( QLocale().decimalPoint(), '.' ); QCOMPARE( QLocale().numberOptions() & QLocale::NumberOption::OmitGroupSeparator, QLocale::NumberOption::DefaultNumberOptions ); QCOMPARE( doubleFieldNoPrec.displayString( 599999898999.0 ), QString( "599,999,898,999" ) ); @@ -514,7 +513,8 @@ void TestQgsField::convertCompatible() QCOMPARE( stringVar.type(), QVariant::String ); QCOMPARE( stringVar.toString(), QString( "lon" ) ); - //double with ',' as decimal separator + //double with ',' as decimal separator for German locale + QLocale::setDefault( QLocale::German ); QVariant doubleCommaVar( "1,2345" ); QVERIFY( doubleField.convertCompatible( doubleCommaVar ) ); QCOMPARE( doubleCommaVar.type(), QVariant::Double ); From bf810e9c50050c597576044e2b930915b0bb2811 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 18:42:14 +0200 Subject: [PATCH 10/13] Check if conversion was successful before passing to locale --- src/core/qgsfield.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp index 098908950562..0b056b1aa829 100644 --- a/src/core/qgsfield.cpp +++ b/src/core/qgsfield.cpp @@ -244,11 +244,14 @@ QString QgsField::displayString( const QVariant &v ) const return QString::number( v.toDouble(), 'f', d->precision ); } } - // Other numeric types out of doubles + // Other numeric types than doubles else if ( isNumeric() && ! QLocale().numberOptions() & QLocale::NumberOption::OmitGroupSeparator ) { - return QLocale().toString( v.toLongLong() ); + bool ok; + qlonglong converted( v.toLongLong( &ok ) ); + if ( ok ) + return QLocale().toString( converted ); } // Fallback if special rules do not apply return v.toString(); @@ -305,7 +308,7 @@ bool QgsField::convertCompatible( QVariant &v ) const QVariant tmp( v ); if ( d->type == QVariant::Double && !tmp.convert( d->type ) ) { - v = v.toString().replace( ',', '.' ); + v = v.toString().replace( QLocale().decimalPoint(), '.' ); } } From cb3eb961381ff1060076d6b991b43c01815b561c Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 9 Jun 2018 18:42:25 +0200 Subject: [PATCH 11/13] Fix test, because we only accept comma for compatible locales --- tests/src/python/test_qgsfieldvalidator.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/src/python/test_qgsfieldvalidator.py b/tests/src/python/test_qgsfieldvalidator.py index e1e4c36190d3..ce4a3a5c2357 100644 --- a/tests/src/python/test_qgsfieldvalidator.py +++ b/tests/src/python/test_qgsfieldvalidator.py @@ -59,21 +59,26 @@ def _test(value, expected): # Valid _test('0.1234', QValidator.Acceptable) - _test('0,1234', QValidator.Acceptable) + + # Apparently we accept comma only when locale say so + if DECIMAL_SEPARATOR != '.': + _test('0,1234', QValidator.Acceptable) # If precision is > 0, regexp validator is used (and it does not support sci notation) if field.precision() == 0: _test('12345.1234e+123', QValidator.Acceptable) _test('12345.1234e-123', QValidator.Acceptable) - _test('12345,1234e+123', QValidator.Acceptable) - _test('12345,1234e-123', QValidator.Acceptable) + if DECIMAL_SEPARATOR != '.': + _test('12345,1234e+123', QValidator.Acceptable) + _test('12345,1234e-123', QValidator.Acceptable) _test('', QValidator.Acceptable) # Out of range _test('12345.1234e+823', QValidator.Intermediate) _test('12345.1234e-823', QValidator.Intermediate) - _test('12345,1234e+823', QValidator.Intermediate) - _test('12345,1234e-823', QValidator.Intermediate) + if DECIMAL_SEPARATOR != '.': + _test('12345,1234e+823', QValidator.Intermediate) + _test('12345,1234e-823', QValidator.Intermediate) # Invalid _test('12345-1234', QValidator.Invalid) From 85b9d37fa82b93db2affbf7a205172be1c57ebd1 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sun, 10 Jun 2018 16:30:20 +0200 Subject: [PATCH 12/13] Use QLocale to parse string representation of numbers Because we now might have thousand separators, also fixes the NULL issue reported by DelazJ. Comes with many test cases. --- src/core/qgsfield.cpp | 72 +++++++++++++++++++++++---- tests/src/core/testqgsfield.cpp | 87 +++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 10 deletions(-) diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp index 0b056b1aa829..7e4c4fb74b15 100644 --- a/src/core/qgsfield.cpp +++ b/src/core/qgsfield.cpp @@ -277,6 +277,68 @@ bool QgsField::convertCompatible( QVariant &v ) const return false; } + // Give it a chance to convert to double since for not '.' locales + // we accept both comma and dot as decimal point + if ( d->type == QVariant::Double && v.type() == QVariant::String ) + { + QVariant tmp( v ); + if ( !tmp.convert( d->type ) ) + { + // This might be a string with thousand separator: use locale to convert + bool ok; + double d = QLocale().toDouble( v.toString(), &ok ); + if ( ok ) + { + v = QVariant( d ); + return true; + } + // For not 'dot' locales, we also want to accept '.' + if ( QLocale().decimalPoint() != '.' ) + { + d = QLocale( QLocale::English ).toDouble( v.toString(), &ok ); + if ( ok ) + { + v = QVariant( d ); + return true; + } + } + } + } + + // For string representation of an int we also might have thousand separator + if ( d->type == QVariant::Int && v.type() == QVariant::String ) + { + QVariant tmp( v ); + if ( !tmp.convert( d->type ) ) + { + // This might be a string with thousand separator: use locale to convert + bool ok; + int i = QLocale().toInt( v.toString(), &ok ); + if ( ok ) + { + v = QVariant( i ); + return true; + } + } + } + + // For string representation of a long we also might have thousand separator + if ( d->type == QVariant::LongLong && v.type() == QVariant::String ) + { + QVariant tmp( v ); + if ( !tmp.convert( d->type ) ) + { + // This might be a string with thousand separator: use locale to convert + bool ok; + qlonglong l = QLocale().toLongLong( v.toString(), &ok ); + if ( ok ) + { + v = QVariant( l ); + return true; + } + } + } + //String representations of doubles in QVariant will return false to convert( QVariant::Int ) //work around this by first converting to double, and then checking whether the double is convertible to int if ( d->type == QVariant::Int && v.canConvert( QVariant::Double ) ) @@ -301,16 +363,6 @@ bool QgsField::convertCompatible( QVariant &v ) const return true; } - // Give it a chance to convert to double since for not '.' locales - // we accept both comma and dot as decimal point - if ( d->type == QVariant::Double && QLocale().decimalPoint() != '.' ) - { - QVariant tmp( v ); - if ( d->type == QVariant::Double && !tmp.convert( d->type ) ) - { - v = v.toString().replace( QLocale().decimalPoint(), '.' ); - } - } if ( !v.convert( d->type ) ) { diff --git a/tests/src/core/testqgsfield.cpp b/tests/src/core/testqgsfield.cpp index a3ffc02abdd5..3af6386ac582 100644 --- a/tests/src/core/testqgsfield.cpp +++ b/tests/src/core/testqgsfield.cpp @@ -498,6 +498,44 @@ void TestQgsField::convertCompatible() QCOMPARE( longlong.type(), QVariant::LongLong ); QCOMPARE( longlong, QVariant( 99999999999999999LL ) ); + //string representation of an int + QVariant stringInt( "123456" ); + QVERIFY( intField.convertCompatible( stringInt ) ); + QCOMPARE( stringInt.type(), QVariant::Int ); + QCOMPARE( stringInt, QVariant( 123456 ) ); + // now with group separator for english locale + stringInt = QVariant( "123,456" ); + QVERIFY( intField.convertCompatible( stringInt ) ); + QCOMPARE( stringInt.type(), QVariant::Int ); + QCOMPARE( stringInt, QVariant( "123456" ) ); + + //string representation of a longlong + QVariant stringLong( "99999999999999999" ); + QVERIFY( longlongField.convertCompatible( stringLong ) ); + QCOMPARE( stringLong.type(), QVariant::LongLong ); + QCOMPARE( stringLong, QVariant( 99999999999999999LL ) ); + // now with group separator for english locale + stringLong = QVariant( "99,999,999,999,999,999" ); + QVERIFY( longlongField.convertCompatible( stringLong ) ); + QCOMPARE( stringLong.type(), QVariant::LongLong ); + QCOMPARE( stringLong, QVariant( 99999999999999999LL ) ); + + + //string representation of a double + QVariant stringDouble( "123456.012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 123456.012345 ) ); + // now with group separator for english locale + stringDouble = QVariant( "1,223,456.012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 1223456.012345 ) ); + // This should not convert + stringDouble = QVariant( "1.223.456,012345" ); + QVERIFY( ! doubleField.convertCompatible( stringDouble ) ); + + //double with precision QgsField doubleWithPrecField( QStringLiteral( "double" ), QVariant::Double, QStringLiteral( "double" ), 10, 3 ); doubleVar = QVariant( 10.12345678 ); @@ -513,12 +551,61 @@ void TestQgsField::convertCompatible() QCOMPARE( stringVar.type(), QVariant::String ); QCOMPARE( stringVar.toString(), QString( "lon" ) ); + + ///////////////////////////////////////////////////////// + // German locale tests + //double with ',' as decimal separator for German locale QLocale::setDefault( QLocale::German ); QVariant doubleCommaVar( "1,2345" ); QVERIFY( doubleField.convertCompatible( doubleCommaVar ) ); QCOMPARE( doubleCommaVar.type(), QVariant::Double ); QCOMPARE( doubleCommaVar.toString(), QString( "1.2345" ) ); + + //string representation of an int + stringInt = QVariant( "123456" ); + QVERIFY( intField.convertCompatible( stringInt ) ); + QCOMPARE( stringInt.type(), QVariant::Int ); + QCOMPARE( stringInt, QVariant( 123456 ) ); + // now with group separator for german locale + stringInt = QVariant( "123.456" ); + QVERIFY( intField.convertCompatible( stringInt ) ); + QCOMPARE( stringInt.type(), QVariant::Int ); + QCOMPARE( stringInt, QVariant( "123456" ) ); + + //string representation of a longlong + stringLong = QVariant( "99999999999999999" ); + QVERIFY( longlongField.convertCompatible( stringLong ) ); + QCOMPARE( stringLong.type(), QVariant::LongLong ); + QCOMPARE( stringLong, QVariant( 99999999999999999LL ) ); + // now with group separator for german locale + stringLong = QVariant( "99.999.999.999.999.999" ); + QVERIFY( longlongField.convertCompatible( stringLong ) ); + QCOMPARE( stringLong.type(), QVariant::LongLong ); + QCOMPARE( stringLong, QVariant( 99999999999999999LL ) ); + + //string representation of a double + stringDouble = QVariant( "123456,012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 123456.012345 ) ); + // For doubles we also want to accept dot as a decimal point + stringDouble = QVariant( "123456.012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 123456.012345 ) ); + // now with group separator for german locale + stringDouble = QVariant( "1.223.456,012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 1223456.012345 ) ); + // Be are good citizens and we also accept english locale + stringDouble = QVariant( "1,223,456.012345" ); + QVERIFY( doubleField.convertCompatible( stringDouble ) ); + QCOMPARE( stringDouble.type(), QVariant::Double ); + QCOMPARE( stringDouble, QVariant( 1223456.012345 ) ); + + } void TestQgsField::dataStream() From d9afb899a0311f4867e0ec5a2cd1265a910a516d Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Mon, 11 Jun 2018 07:19:45 +0200 Subject: [PATCH 13/13] [bugfix] Fix Z&M edition. Fix double conversion from locale. Mnually Cherry-picked from 0ba36acf93353ff177f4e149215dbee8b9d5d78d --- src/app/vertextool/qgsvertexeditor.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/app/vertextool/qgsvertexeditor.cpp b/src/app/vertextool/qgsvertexeditor.cpp index e31232f3a177..aedf14ad3628 100644 --- a/src/app/vertextool/qgsvertexeditor.cpp +++ b/src/app/vertextool/qgsvertexeditor.cpp @@ -199,15 +199,23 @@ bool QgsVertexEditorModel::setData( const QModelIndex &index, const QVariant &va return false; } - double x = ( index.column() == 0 ? value.toDouble() : mSelectedFeature->vertexMap().at( index.row() )->point().x() ); - double y = ( index.column() == 1 ? value.toDouble() : mSelectedFeature->vertexMap().at( index.row() )->point().y() ); + // Get double value wrt current locale. + bool ok; + double doubleValue = QLocale().toDouble( value.toString(), &ok ); + // If not valid and locale's decimal point is not '.' let's try with english locale + if ( ! ok && QLocale().decimalPoint() != '.' ) + { + doubleValue = QLocale( QLocale::English ).toDouble( value.toString() ); + } + + double x = ( index.column() == 0 ? doubleValue : mSelectedFeature->vertexMap().at( index.row() )->point().x() ); + double y = ( index.column() == 1 ? doubleValue : mSelectedFeature->vertexMap().at( index.row() )->point().y() ); if ( index.column() == mRCol ) // radius modified { if ( index.row() == 0 || index.row() >= mSelectedFeature->vertexMap().count() - 1 ) return false; - double r = value.toDouble(); double x1 = mSelectedFeature->vertexMap().at( index.row() - 1 )->point().x(); double y1 = mSelectedFeature->vertexMap().at( index.row() - 1 )->point().y(); double x2 = x; @@ -216,7 +224,7 @@ bool QgsVertexEditorModel::setData( const QModelIndex &index, const QVariant &va double y3 = mSelectedFeature->vertexMap().at( index.row() + 1 )->point().y(); QgsPoint result; - if ( QgsGeometryUtils::segmentMidPoint( QgsPoint( x1, y1 ), QgsPoint( x3, y3 ), result, r, QgsPoint( x2, y2 ) ) ) + if ( QgsGeometryUtils::segmentMidPoint( QgsPoint( x1, y1 ), QgsPoint( x3, y3 ), result, doubleValue, QgsPoint( x2, y2 ) ) ) { x = result.x(); y = result.y();