Config: make bool and int parsing explicit via functions

Passing a tuple to the mapping interface isn't the best of interfaces,
as the key is only the string.

Instead, expose `Config.get_bool()` and `Config.get_int()` methods to
parse the values as per the git-config rules before returning the
appropriate type to the user.

The mapping interface itself returns a string.

Author: carlosmn

docs/config.rst

@@ -24,18 +24,9 @@ The Config type
 
 The :class:`Config` Mapping interface.
 
-Parsing the values
-===================
+When using the mapping interface, the value is returned as a
+string. In order to apply the git-config parsing rules, you can use
+:method:`Config.get_bool` or :method:`Config.get_int`.
 
-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.
+.. automethod:: pygit2.Config.get_bool
+.. automethod:: pygit2.Config.get_int

src/config.c

@@ -168,70 +168,94 @@ Config_contains(Config *self, PyObject *py_key) {
     return 1;
 }
 
-
-PyObject *
-Config_getitem(Config *self, PyObject *py_input_key)
+/* Get the C string value given a python string as key */
+static int
+get_string(const char **key_out, Config *self, PyObject *py_key)
 {
-    int err;
-    const char *value_str;
+    PyObject *tkey;
     const char *key;
-    PyObject *py_key, *py_value, *tkey, *tmp_type = NULL;
-    PyTypeObject *py_type = NULL;
+    int err;
 
-    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;
+    key = py_str_borrow_c_str(&tkey, py_key, NULL);
+    if (key == NULL)
+        return -1;
+
+    err = git_config_get_string(key_out, self->config, key);
+    Py_CLEAR(tkey);
+
+    if (err == GIT_ENOTFOUND) {
+        PyErr_SetObject(PyExc_KeyError, py_key);
+        return -1;
     }
 
-    /* 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;
+    if (err < 0) {
+        Error_set(err);
+        return -1;
     }
 
-    key = py_str_borrow_c_str(&tkey, py_key, NULL);
-    if (key == NULL)
+    return 0;
+}
+
+PyObject *
+Config_getitem(Config *self, PyObject *py_key)
+{
+    int err;
+    const char *value_str;
+
+    err = get_string(&value_str, self, py_key);
+    if (err < 0)
         return NULL;
 
-    err = git_config_get_string(&value_str, self->config, key);
-    Py_CLEAR(tkey);
+    return to_unicode(value_str, NULL, NULL);
+}
+
+PyDoc_STRVAR(Config_get_bool__doc__,
+  "get_bool(key) -> Bool\n"
+  "\n"
+  "Look up *key* and parse its value as a boolean as per the git-config rules\n"
+  "\n"
+  "Truthy values are: 'true', 1, 'on' or 'yes'. Falsy values are: 'false',\n"
+  "0, 'off' and 'no'");
+
+PyObject *
+Config_get_bool(Config *self, PyObject *key)
+{
+    int err, value;
+    const char *value_str;
+
+    err = get_string(&value_str, self, key);
     if (err < 0)
-        goto cleanup;
-
-    /* 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);
-    }
+        return NULL;
 
-cleanup:
-    if (err < 0) {
-        if (err == GIT_ENOTFOUND) {
-            PyErr_SetObject(PyExc_KeyError, py_key);
-            return NULL;
-        }
+    if ((err = git_config_parse_bool(&value, value_str)) < 0)
+        return NULL;
 
-        return Error_set(err);
-    }
+    return PyBool_FromLong(value);
+}
+
+PyDoc_STRVAR(Config_get_int__doc__,
+  "get_int(key) -> int\n"
+  "\n"
+  "Look up *key* and parse its value as an integer as per the git-config rules\n"
+  "\n"
+  "A value can have a suffix 'k', 'm' or 'g' which stand for 'kilo', 'mega' and\n"
+  "'giga' respectively");
+
+PyObject *
+Config_get_int(Config *self, PyObject *key)
+{
+    int err;
+    int64_t value;
+    const char *value_str;
+
+    err = get_string(&value_str, self, key);
+    if (err < 0)
+        return NULL;
+
+    if ((err = git_config_parse_int64(&value, value_str)) < 0)
+        return NULL;
 
-    return py_value;
+    return PyLong_FromLongLong(value);
 }
 
 int
@@ -396,6 +420,8 @@ PyMethodDef Config_methods[] = {
     METHOD(Config, add_file, METH_VARARGS | METH_KEYWORDS),
     METHOD(Config, get_multivar, METH_VARARGS),
     METHOD(Config, set_multivar, METH_VARARGS),
+    METHOD(Config, get_bool, METH_O),
+    METHOD(Config, get_int, METH_O),
     {NULL}
 };
 

test/test_config.py

@@ -74,7 +74,7 @@ def test_new(self):
 
         config_read = Config(CONFIG_FILENAME)
         self.assertTrue('core.bare' in config_read)
-        self.assertFalse(config_read['core.bare',bool])
+        self.assertFalse(config_read.get_bool('core.bare'))
         self.assertTrue('core.editor' in config_read)
         self.assertEqual(config_read['core.editor'], 'ed')
 
@@ -88,9 +88,9 @@ def test_add(self):
 
         config.add_file(CONFIG_FILENAME, 0)
         self.assertTrue('this.that' in config)
-        self.assertTrue(config['this.that',bool])
+        self.assertTrue(config.get_bool('this.that'))
         self.assertTrue('something.other.here' in config)
-        self.assertFalse(config['something.other.here',bool])
+        self.assertFalse(config.get_bool('something.other.here'))
 
     def test_read(self):
         config = self.repo.config
@@ -103,11 +103,11 @@ def test_read(self):
                                  lambda: config['abc.def'])
 
         self.assertTrue('core.bare' in config)
-        self.assertFalse(config['core.bare',bool])
+        self.assertFalse(config.get_bool('core.bare'))
         self.assertTrue('core.editor' in config)
         self.assertEqual(config['core.editor'], 'ed')
         self.assertTrue('core.repositoryformatversion' in config)
-        self.assertEqual(config['core.repositoryformatversion',int], 0)
+        self.assertEqual(config.get_int('core.repositoryformatversion'), 0)
 
         new_file = open(CONFIG_FILENAME, "w")
         new_file.write("[this]\n\tthat = foobar\n\tthat = foobeer\n")
@@ -129,7 +129,7 @@ def test_write(self):
         self.assertFalse('core.dummy1' in config)
         config['core.dummy1'] = 42
         self.assertTrue('core.dummy1' in config)
-        self.assertEqual(config['core.dummy1',int], 42)
+        self.assertEqual(config.get_int('core.dummy1'), 42)
 
         self.assertFalse('core.dummy2' in config)
         config['core.dummy2'] = 'foobar'