From 687dc5388e7f75ee71ea683006ddd122d0fd2647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 27 Mar 2014 16:53:16 +0100 Subject: [PATCH] config: make type conversion explicit The type of a config value depends on the tool that interprets it. Parsing eagerly can lead to a situation where we return a bool instead of a string or a number. Let the user specify the type themselves by passing in a (str, type) tuple into the mapping interface. --- docs/config.rst | 16 +++++++++++++++ src/config.c | 49 +++++++++++++++++++++++++++++++++++---------- src/utils.h | 2 ++ test/test_config.py | 12 +++++------ 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 348c347..4bdaaa1 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -23,3 +23,19 @@ The Config type set multiple times in the configuration files. The :class:`Config` Mapping interface. + +Parsing the values +=================== + +Instead of a string, a tuple of `(str,type)` can be used to look up a +key and parse it through the Git rules. E.g. + + config['core.bare',bool] + +will return True if 'core.bare' is truthy. + +Truty values are: 'true', 1, 'on' or 'yes'. Falsy values are: 'false', +0, 'off' and 'no'. + +Available types are `bool` and `int`. Not specifying a type returns a +string. diff --git a/src/config.c b/src/config.c index 9b2625d..58fdeaa 100644 --- a/src/config.c +++ b/src/config.c @@ -170,29 +170,56 @@ Config_contains(Config *self, PyObject *py_key) { PyObject * -Config_getitem(Config *self, PyObject *py_key) +Config_getitem(Config *self, PyObject *py_input_key) { - int64_t value_int; - int err, value_bool; + int err; const char *value_str; const char *key; - PyObject* py_value, *tmp; + PyObject *py_key, *py_value, *tkey, *tmp_type = NULL; + PyTypeObject *py_type = NULL; - key = py_str_borrow_c_str(&tmp, py_key, NULL); + if (PyTuple_Check(py_input_key) && PyTuple_Size(py_input_key) == 2) { + py_key = PyTuple_GetItem(py_input_key, 0); + tmp_type = PyTuple_GetItem(py_input_key, 1); + } else { + py_key = py_input_key; + } + + /* If passed a tuple, make sure the second item is a type */ + if (tmp_type) { + if (!PyType_Check(tmp_type)) + return NULL; + else + py_type = (PyTypeObject *) tmp_type; + } + + key = py_str_borrow_c_str(&tkey, py_key, NULL); if (key == NULL) return NULL; err = git_config_get_string(&value_str, self->config, key); - Py_CLEAR(tmp); + Py_CLEAR(tkey); if (err < 0) goto cleanup; - if (git_config_parse_int64(&value_int, value_str) == 0) - py_value = PyLong_FromLongLong(value_int); - else if(git_config_parse_bool(&value_bool, value_str) == 0) - py_value = PyBool_FromLong(value_bool); - else + /* If the user specified a type, let's parse it */ + if (py_type) { + if (py_type == &PyBool_Type) { + int value; + if ((err = git_config_parse_bool(&value, value_str)) < 0) + goto cleanup; + + py_value = PyBool_FromLong(value); + } else if (py_type == &PyInteger_Type) { + int64_t value; + if ((err = git_config_parse_int64(&value, value_str)) < 0) + goto cleanup; + + py_value = PyLong_FromLongLong(value); + } + } else { py_value = to_unicode(value_str, NULL, NULL); + } cleanup: if (err < 0) { diff --git a/src/utils.h b/src/utils.h index 81744f6..e321ea1 100644 --- a/src/utils.h +++ b/src/utils.h @@ -47,6 +47,7 @@ #undef PyLong_Check #define PyLong_Check PyInt_Check #define PyLong_FromLong PyInt_FromLong + #define PyInteger_Type PyInt_Type #define PyBytes_AS_STRING PyString_AS_STRING #define PyBytes_AsString PyString_AsString #define PyBytes_AsStringAndSize PyString_AsStringAndSize @@ -57,6 +58,7 @@ #define to_path(x) to_bytes(x) #define to_encoding(x) to_bytes(x) #else + #define PyInteger_Type PyLong_Type #define to_path(x) to_unicode(x, Py_FileSystemDefaultEncoding, "strict") #define to_encoding(x) PyUnicode_DecodeASCII(x, strlen(x), "strict") #endif diff --git a/test/test_config.py b/test/test_config.py index 9f4d460..f29c703 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -74,7 +74,7 @@ class ConfigTest(utils.RepoTestCase): config_read = Config(CONFIG_FILENAME) self.assertTrue('core.bare' in config_read) - self.assertFalse(config_read['core.bare']) + self.assertFalse(config_read['core.bare',bool]) self.assertTrue('core.editor' in config_read) self.assertEqual(config_read['core.editor'], 'ed') @@ -88,9 +88,9 @@ class ConfigTest(utils.RepoTestCase): config.add_file(CONFIG_FILENAME, 0) self.assertTrue('this.that' in config) - self.assertTrue(config['this.that']) + self.assertTrue(config['this.that',bool]) self.assertTrue('something.other.here' in config) - self.assertFalse(config['something.other.here']) + self.assertFalse(config['something.other.here',bool]) def test_read(self): config = self.repo.config @@ -103,11 +103,11 @@ class ConfigTest(utils.RepoTestCase): lambda: config['abc.def']) self.assertTrue('core.bare' in config) - self.assertFalse(config['core.bare']) + self.assertFalse(config['core.bare',bool]) self.assertTrue('core.editor' in config) self.assertEqual(config['core.editor'], 'ed') self.assertTrue('core.repositoryformatversion' in config) - self.assertEqual(config['core.repositoryformatversion'], 0) + self.assertEqual(config['core.repositoryformatversion',int], 0) new_file = open(CONFIG_FILENAME, "w") new_file.write("[this]\n\tthat = foobar\n\tthat = foobeer\n") @@ -129,7 +129,7 @@ class ConfigTest(utils.RepoTestCase): self.assertFalse('core.dummy1' in config) config['core.dummy1'] = 42 self.assertTrue('core.dummy1' in config) - self.assertEqual(config['core.dummy1'], 42) + self.assertEqual(config['core.dummy1',int], 42) self.assertFalse('core.dummy2' in config) config['core.dummy2'] = 'foobar'