From cbd0ece0a6a6a9a7ea2a134d77160879a07efaf1 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 31 Jan 2018 12:34:34 +0100 Subject: [PATCH 1/2] Support comma in annotations when merging inline --- scripts/sipify.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/sipify.pl b/scripts/sipify.pl index d1590e45b72..0dcff3d3bbd 100755 --- a/scripts/sipify.pl +++ b/scripts/sipify.pl @@ -343,9 +343,9 @@ sub fix_annotations { $line =~ s/SIP_VIRTUALERRORHANDLER\(\s*(\w+)\s*\)/\/VirtualErrorHandler=$1\//; # combine multiple annotations - # https://regex101.com/r/uvCt4M/3 + # https://regex101.com/r/uvCt4M/4 do {no warnings 'uninitialized'; - $line =~ s/\/(\w+(=\w+)?)\/\s*\/(\w+(=\w+)?)\//\/$1,$3\//; + $line =~ s/\/([\w,]+(=\w+)?)\/\s*\/([\w,]+(=\w+)?)\//\/$1,$3\//; (! $3) or dbg_info("combine multiple annotations -- works only for 2"); }; From cc1625c83ec50c698a6cf79e7da617efd164b287 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 31 Jan 2018 12:43:02 +0100 Subject: [PATCH 2/2] [bugfix][attrtable] Convert comma to dot for floating point input This fixes an unreported bug that without detecting an invalid input when using a comma as a decimal separator silently converts the entered value to NULL. Since locale support in QGIS is in its early stages we convert commas to dots within the validator, this is common practice in almost all web applications where you can enter a comma instead of a dot and the conversion appears while you digit. This comes with brand new tests for QgsFieldValidator. Bonus: small fix in sipify. --- python/gui/qgsfieldvalidator.sip.in | 2 +- src/gui/qgsfieldvalidator.cpp | 7 +- src/gui/qgsfieldvalidator.h | 2 +- tests/src/python/CMakeLists.txt | 1 + tests/src/python/test_qgsfieldvalidator.py | 105 +++++++++++++++++++++ tests/testdata/bug_17878.gpkg | Bin 0 -> 118784 bytes 6 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 tests/src/python/test_qgsfieldvalidator.py create mode 100644 tests/testdata/bug_17878.gpkg diff --git a/python/gui/qgsfieldvalidator.sip.in b/python/gui/qgsfieldvalidator.sip.in index 97b6c162e7d..c50ccc13e65 100644 --- a/python/gui/qgsfieldvalidator.sip.in +++ b/python/gui/qgsfieldvalidator.sip.in @@ -19,7 +19,7 @@ class QgsFieldValidator : QValidator QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" ); ~QgsFieldValidator(); - virtual State validate( QString &s /Constrained/, int &i /In,Out/ ) const; + virtual State validate( QString &s /Constrained,In,Out/, int &i /In,Out/ ) const; virtual void fixup( QString &s /Constrained/ ) const; diff --git a/src/gui/qgsfieldvalidator.cpp b/src/gui/qgsfieldvalidator.cpp index 601f8ebc662..bcc3ebe0e56 100644 --- a/src/gui/qgsfieldvalidator.cpp +++ b/src/gui/qgsfieldvalidator.cpp @@ -84,7 +84,6 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co mValidator = nullptr; } - QgsSettings settings; mNullValue = QgsApplication::nullRepresentation(); } @@ -113,6 +112,12 @@ 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/src/gui/qgsfieldvalidator.h b/src/gui/qgsfieldvalidator.h index bcdec8923e5..dda5c17c08c 100644 --- a/src/gui/qgsfieldvalidator.h +++ b/src/gui/qgsfieldvalidator.h @@ -38,7 +38,7 @@ class GUI_EXPORT QgsFieldValidator : public QValidator QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" ); ~QgsFieldValidator() override; - State validate( QString &s SIP_CONSTRAINED, int &i SIP_INOUT ) const override; + State validate( QString &s SIP_CONSTRAINED SIP_INOUT, int &i SIP_INOUT ) const override; void fixup( QString &s SIP_CONSTRAINED ) const override; QString dateFormat() const { return mDateFormat; } diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt index 8ab4626d5f4..4328456595d 100644 --- a/tests/src/python/CMakeLists.txt +++ b/tests/src/python/CMakeLists.txt @@ -196,6 +196,7 @@ ADD_PYTHON_TEST(PyQgsAuthManagerProxy test_authmanager_proxy.py) ADD_PYTHON_TEST(PyQgsAuthSettingsWidget test_authsettingswidget.py) ADD_PYTHON_TEST(PyQgsAuxiliaryStorage test_qgsauxiliarystorage.py) ADD_PYTHON_TEST(PyQgsAuthManagerOgr test_authmanager_ogr.py) +ADD_PYTHON_TEST(PyQgsFieldValidator test_qgsfieldvalidator.py) IF (NOT WIN32) ADD_PYTHON_TEST(PyQgsLogger test_qgslogger.py) diff --git a/tests/src/python/test_qgsfieldvalidator.py b/tests/src/python/test_qgsfieldvalidator.py new file mode 100644 index 00000000000..607277194d8 --- /dev/null +++ b/tests/src/python/test_qgsfieldvalidator.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8 -*- +"""QGIS Unit tests for QgsFieldValidator. + +.. note:: This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +""" +__author__ = 'Alessandro Pasotti' +__date__ = '31/01/2018' +__copyright__ = 'Copyright 2018, The QGIS Project' +# This will get replaced with a git SHA1 when you do a git archive +__revision__ = '$Format:%H$' + +import qgis # NOQA + +from qgis.PyQt.QtCore import QVariant +from qgis.PyQt.QtGui import QValidator +from qgis.core import QgsVectorLayer +from qgis.gui import QgsFieldValidator +from qgis.testing import start_app, unittest +from utilities import unitTestDataPath + +TEST_DATA_DIR = unitTestDataPath() + + +start_app() + + +class TestQgsFieldValidator(unittest.TestCase): + + def setUp(self): + """Run before each test.""" + testPath = TEST_DATA_DIR + '/' + 'bug_17878.gpkg|layername=bug_17878' + self.vl = QgsVectorLayer(testPath, "test_data", "ogr") + assert self.vl.isValid() + + def tearDown(self): + """Run after each test.""" + pass + + def test_validator(self): + # Test the double + """ + 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. + """ + + 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', '') + + def _test(value, expected): + ret = validator.validate(value, 0) + self.assertEqual(ret[0], expected, value) + 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) + + # Invalid + _test('12345-1234', QValidator.Invalid) + _test('onetwothree', QValidator.Invalid) + + int_field = self.vl.fields()[self.vl.fields().indexFromName('int_field')] + self.assertEqual(int_field.precision(), 0) # this is what the provider reports :( + self.assertEqual(int_field.length(), 0) # not set + self.assertEqual(int_field.type(), QVariant.Int) + + validator = QgsFieldValidator(None, int_field, '0', '') + + # Valid + _test('0', QValidator.Acceptable) + _test('1234', QValidator.Acceptable) + _test('', QValidator.Acceptable) + + # Invalid + _test('12345-1234', QValidator.Invalid) + _test('12345.1234', QValidator.Invalid) + _test('onetwothree', QValidator.Invalid) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/testdata/bug_17878.gpkg b/tests/testdata/bug_17878.gpkg new file mode 100644 index 0000000000000000000000000000000000000000..4f410bc9117c524625efa552cfdf400365681a42 GIT binary patch literal 118784 zcmeI5OK=;>dB*`#B1nl3?L!Oia%Hx#mO)q`MUaw6N~<`KID%l20ELIOTt%6TfCf1c zFvA4~OL8rzOj6QT5}%xkQ{{`DDj)KwN+qdE;+%5GAyrA`C<{RYkJ)N*3Vy!QOQ7nu0j4^A@26C?kodYv2j?~(r;`I&y>35#10009sH z0T2KI5C8!X009sH0T2Lzqakqi0!!yj!oJSD!@fq&46*Otp&tf@6GJ~7{x-Y82LwO> z1V8`;KmY_l00ck)1VEr$2>ePQbR%%Kf2>xN%CsU%v`kj1yjmC5*TfRZ=kl3shHILB z)`c@rNtWwnQ4-jj;X19*I+aS4G`1QFtv&dK`3YSQoDGgWwx3cG>I!X$f@D7A7gpld z1+fF|@sOR$?^gyc2F~_89ojy3Kb0qQmsRn~R1r(uaz1tDCn+wQx|M$r(LE#ag=C?) zq}}p~`MJxHcz*c~m&>LX>>yWXX6F(!v+?Qqxyk7(v(wWvSFX;)lf}ZFY%X0`c@R-k zj>IFgQ!`VMhYx*xuFgzfRehFo=_QUmEmfyf+7L@?k@%EP@VWV^gc`h<=Fh009sH0T2KI5C8!X009sH0T2Lz z<4NGl!P8^s#)fXF()B!QuJVc5x!Ji26_jS3HY|tfsl?pGRAOQ#QJ9*)HgombOk#58 z^5qZD{Nxmi7##Wc0Q`E|33?ieC7y&OAr775C8!X009sH0T2KI z5C8!X0D&$iFc|C|>&N^5E-yuh4+09=3_vttx$RH<(*XVaO2Y5(u1 zfsvmc*DOFT5C8!X009sH0T2KI5C8!X009sHfzAn>Ju!C9w)bk7GG6L@`oC%Er{iLFP40F! z$B}d<&*cgvn`w;~B`gHtlDF=VT=qVTkVZIK}wB_E4&RZwVylw_%))CEyet`Q@bTJ}gukR(|lk0`0qMuRA8f^k8x3p-yK360)k7|*xE_=zmn_$vLFR{7?7nTg?rv@gV#6fA_r z^N=;tey&lcEtoI4KH7r$ z*2WI-eR-%aoSz5=M5#Yv;R0#nm;1X%J6j zO?~Mt+TX8T#C36#wa-nyA$~&1Lbj-8$P!8S-17IlmCfV}OebXuWc@L(C^fR2OD`pJ zE95S>;$#qaw9#1H5v{_a)oQ$hmZ~1psf;&;7|C(BnLf(g;_^Cfrq2{ADGdkeSZw$6 zr$XT?SAx&dbSzQE%{KxwUX=qQq~uz*SwhT3v-r zzp%`;HLM%us#0ql8B0$wX=$_UslAh3Kze>#3#YLzD56m1>$Ji*wi;yI2pCJ9?h8M- z(s4Uw1EElOb~gCwnB87Y8_ax3t~P5@Lo0r-V}EN|dxNwu?6{t?U&FQ% zQMT5pUx0RX;}cRyXI9ikWVO0ltCORoSzA*drslHYtbI6IH`Y?t*s&?xm6q@TE(^?FV+nlvnGlv~+!?3E*_-Ypsw@Z%4W8p?E4(P?~khp3;=; z&S8POoh&ZuJx?sz*B35c+7p-axkkHlI)!%M?FogW(crTUJ8!OhnK!+yc{0766>X0U zvC*Qyt4+xrCN7ql(ibZtt?M3o?Twddqf{5y6_JgGwGP+JdNdlZ3JrxXv0?2h>)+Uz zv5;P}dpKeS9A~5BiXztNc=W=`#D&_#g>vD-oog4Cu3e~n7>!3Ic_SK&#hscwPMjJ% zUUMjJ=H*b>o_?XVG1S%Op!OVx(XN(qoj=hRUW)En!e%KL4Z*xUIp5oPRK4PxQBSaG zVZS;x-02N!eeZ6}t{k3zK&$&6x1QFj;t8Sm9zJ8KHX7Bb6XSAyRglC_Sa)RB3`@&8 zy;#GM>t)te`F7uBS`j5tOTtqfz4b1p{{R0|J%vC|VfZhGRtASo{n-HRAL#p!zQ6AM zDxgT4lc_mRMC@ zZK`gK(i*J^Y8YWtmgL$NX~-32L#S(>wEnSJQm<^`?g;g=;VsEhin5`Ku)Zdi7Ay(N ztUH&~Or`2_bKP2mVLgDptYxBMD{SLY3kd7%oj};F{Y(Q0bJKya3j;`$t;jiusEO!& z*p)>|9$ptkU*L!%{c4(sqV=X=|h|AxveoT4#-fjpemfXM3Gb9iwSY z)D`KMti{swTRPj(j@UXBxP!l07hO8s;mdg@BUV(T4pIO_M|M;)ENXgW%kD{q8G z@3Xf}^X=NaF)Gy8xer@W{SvWg+8Wgr?4_wxw-kPFp8N;G#;XAPP2Qc?b++No*dDT? zy2L$Z8r3D+0p=85l-U2@;iMM9KmY_l00ck)1V8`;KmY_l00cmwiwR)=e-{@XLhea!+K{Ymb@WEpHYMBf*~VQhI{RgDS=3}^ zfvCp$9Ej3+Yilgq)az?B@W}IT?jo-iyPeA}k^H^Ibb;fSlKBF&y`~#E)a=aMAO>KKtQO&h zH%XN_vo1##(|4K0r09hxdGf?t2&xnFiN*AN)z6p5^aeI^fClE+VAWdL57WT(1g!?< z-4@vTkj`6_p0sT>@s(;(q=P1Pcu>Pyp}9y#|eO?~k3QVMhF-B9@Y_2Bb_CPSMJqK)$# zW_QAo&8XPkhQd>U)vvFm< zqS}{s7&)=*kTIhq>&(iHq&R$?CgMu&>z&QVWn z-8V5yioKJIJ%4*tH=LC9ZJeex7C&`s7N*PShAFFUI7uI+`sq|NaUW&c`H*GLL9qYO z-+ZpOjagQMm}w%X)gYIpBA8Klx{ycE%@A`k#_xC6M zp!es!f7J86;I{+CV0Pr!hyQUnHuUvlyd3a~i8lD%9_ba!60&n_32etMgu-KE!JYHk zc;iuXl~2sh&CXdDZ<*bnHkOH4?h&?75vhpfh&l9dTRbZkvgr(qy2Lq+L~Dyntm~|A zMaVJ>z>b87u+tsbkboKUi;yL5AzfV3oy&5Q9S~vqu!AcWV^7Dv5E^}Wz5i2}O-ZXK zwue*JAjq#fw?|-R&ip&}_R0D6@zx`KI(hqvN7UWh3Ho>ZmM~V&FAz3NWCJB9(Nt7A25xzDn0W~ZH6%-$r?2Z@R4 zhYzME<{e)X>^`C2tCutSWr1B9L9emPE4r)Wa+0|^lld^JEo(W995NYseeEqddPNJ(9_n1?AXIyDSO+zg3Cd)ktWx#1>v$HVu80hI2Y~GQHsLZ;YuavF%??RjsZ(aHLn_nF6qT2C6K*6iIYsiieY29F zh_XbIl0-Mj;znIm=!7g)x773hf+PPFV1M|400@8p2!H?xfB*=900@8p2!H?xbQ^&$ zo@C1f1N*KPXz%||PXyQ>J|F-BAOHd&00JNY0w4eaP6EHV96S;Doi8|@a18<=00P}g z0Q>*Dwd5c-2!H?xfB*=900@8p2!H?xfB*<|P5|@&&RHQO2!H?xfB*=900@8p2!H?x zfB*<|D*<)>KN9@Cz{sBre}CvZgWnqzPXEK{$y2{I@X!6U?{80jtM|`)zQMfl`M)Kw zUAY_@y*t*wQ!@^;)(t#Z+c|jC`U794RjN?S!*i~AWS@8ulDu8ua%6#9MvhQfF62Dj6S@Tf}L4hGhRI+c`tyx%p)7Rj57Tzl2Y>xo6iCAmx` zSyi-LiVZQAbLpjIZiU?CR^mhtqlPg3W83E^Lg92exP4=9S=mwcV!5rTn(t_bl4^k? z) z^lx9^Q@D0=84`UpohuZRi=?o3DMv^#pU$L+mfE;jj+2@wZL)7+Q~egDE$wKZ z;d>W@0nIj9&GVmqPn(C7*q8J{SlumZWg%*eh(b|+61vtuG>N^Vn