From 0395817102e9b8601c93e60349360fcdd8a3c66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=C3=A1n=20Maureira-Fredes?= Date: Mon, 28 Oct 2019 12:24:48 +0100 Subject: Fix booleans and empty list cases in QSettings After the fix for PYSIDE-1010 there were two things that were not properly understood: 1. A special case for booleans was required, 2. When a list was detected, the split was wrongly creating a list with a '0' instead of an empty one. Additonally, due to the wrong treatment we couldn't notice if the value 0 was None, the number zero, or even false, so this patch amends the previous implementation to properly treat these cases. New test cases were added. Change-Id: I41d5387bb835cfa96f94e5577e993a4b87b303f0 Fixes: PYSIDE-1130 Fixes: PYSIDE-820 Reviewed-by: Cristian Maureira-Fredes (cherry picked from commit ba04613a65be7b50bbcc9dae8c18d195243513c3) Reviewed-by: Christian Tismer --- sources/pyside2/PySide2/glue/qtcore.cpp | 43 ++++++++++++++++-------- sources/pyside2/tests/QtCore/qsettings_test.py | 46 ++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/sources/pyside2/PySide2/glue/qtcore.cpp b/sources/pyside2/PySide2/glue/qtcore.cpp index 93f7321aa..a218e433f 100644 --- a/sources/pyside2/PySide2/glue/qtcore.cpp +++ b/sources/pyside2/PySide2/glue/qtcore.cpp @@ -57,24 +57,39 @@ bool py2kStrCheck(PyObject *obj) // @snippet pystring-check // @snippet qsettings-value -QVariant out = %CPPSELF.value(%1, %2); +// If we enter the kwds, means that we have a defaultValue or +// at least a type. +// This avoids that we are passing '0' as defaultValue. +// defaultValue can also be passed as positional argument, +// not only as keyword. +QVariant out; +if (kwds || numArgs > 1) + out = %CPPSELF.value(%1, %2); +else + out = %CPPSELF.value(%1); + PyTypeObject *typeObj = reinterpret_cast(%PYARG_3); if (typeObj) { if (typeObj == &PyList_Type) { - QByteArrayList valuesList = out.toByteArray().split(','); - const int valuesSize = valuesList.size(); - if (valuesSize > 0) { - PyObject *list = PyList_New(valuesSize); - for (int i = 0; i < valuesSize; i++) { - PyObject *item = PyUnicode_FromString(valuesList[i].data()); - PyList_SET_ITEM(list, i, item); - Py_DECREF(item); - } - %PYARG_0 = list; + QByteArray out_ba = out.toByteArray(); + if (!out_ba.isEmpty()) { + QByteArrayList valuesList = out_ba.split(','); + const int valuesSize = valuesList.size(); + if (valuesSize > 0) { + PyObject *list = PyList_New(valuesSize); + for (int i = 0; i < valuesSize; i++) { + PyObject *item = PyUnicode_FromString(valuesList[i].data()); + PyList_SET_ITEM(list, i, item); + Py_DECREF(item); + } + %PYARG_0 = list; + } else { + %PYARG_0 = %CONVERTTOPYTHON[QVariant](out); + } } else { - %PYARG_0 = %CONVERTTOPYTHON[QVariant](out); + %PYARG_0 = PyList_New(0); } } else if (typeObj == &PyBytes_Type) { QByteArray asByteArray = out.toByteArray(); @@ -94,11 +109,13 @@ if (typeObj) { } else if (typeObj == &PyFloat_Type) { float asFloat = out.toFloat(); %PYARG_0 = PyFloat_FromDouble(asFloat); + } else if (typeObj == &PyBool_Type) { + %PYARG_0 = out.toBool() ? Py_True : Py_False; } // TODO: PyDict_Type and PyTuple_Type } else { - if (out == 0) + if (!out.isValid()) %PYARG_0 = Py_None; else %PYARG_0 = %CONVERTTOPYTHON[QVariant](out); diff --git a/sources/pyside2/tests/QtCore/qsettings_test.py b/sources/pyside2/tests/QtCore/qsettings_test.py index 6d64b0db3..36a4c3c62 100644 --- a/sources/pyside2/tests/QtCore/qsettings_test.py +++ b/sources/pyside2/tests/QtCore/qsettings_test.py @@ -55,15 +55,55 @@ class TestQSettings(unittest.TestCase): def testDefaultValueConversion(self): settings = QSettings('foo.ini', QSettings.IniFormat) - r = settings.value('lala', 22) + settings.setValue('zero_value', 0) + settings.setValue('empty_list', []) + settings.setValue('bool1', False) + settings.setValue('bool2', True) + del settings + + # Loading values already set + settings = QSettings('foo.ini', QSettings.IniFormat) + + # Getting value that doesn't exist + r = settings.value("variable") + self.assertEqual(type(r), type(None)) + + # Handling zero value + r = settings.value('zero_value') if py3k.IS_PY3K: self.assertEqual(type(r), int) else: self.assertEqual(type(r), long) - r = settings.value('lala', 22, type=str) - self.assertEqual(type(r), str) + r = settings.value('zero_value', type=int) + self.assertEqual(type(r), int) + + # Empty list + r = settings.value('empty_list') + self.assertTrue(len(r) == 0) + self.assertEqual(type(r), list) + + r = settings.value('empty_list', type=list) + self.assertTrue(len(r) == 0) + self.assertEqual(type(r), list) + + # Booleans + r = settings.value('bool1') + self.assertEqual(type(r), bool) + + r = settings.value('bool2') + self.assertEqual(type(r), bool) + + r = settings.value('bool1', type=bool) + self.assertEqual(type(r), bool) + + r = settings.value('bool2', type=int) + self.assertEqual(type(r), int) + + r = settings.value('bool2', type=bool) + self.assertEqual(type(r), bool) + # Not set variable, but with default value r = settings.value('lala', 22, type=bytes) self.assertEqual(type(r), bytes) -- cgit v1.2.3