Config: switch from foreach iterator

An iterator is much more natural in python, so let's use that.

docs/config.rst

@@ -10,9 +10,17 @@ The Config type
 
 .. automethod:: pygit2.Config.get_system_config
 .. automethod:: pygit2.Config.get_global_config
-.. automethod:: pygit2.Config.foreach
 .. automethod:: pygit2.Config.add_file
 .. automethod:: pygit2.Config.get_multivar
 .. automethod:: pygit2.Config.set_multivar
 
 The :class:`Config` Mapping interface.
+
+Iterator
+=========
+
+The :class:`Config` class has an iterator which can be used to loop
+through all the entries in the configuration. Each element is a tuple
+containing the name and the value of each configuration variable. Be
+aware that this may return multiple versions of each entry if they are
+set multiple times in the configuration files.

src/config.c

@@ -33,6 +33,7 @@
 #include "config.h"
 
 extern PyTypeObject ConfigType;
+extern PyTypeObject ConfigIterType;
 
 
 PyObject *
@@ -239,70 +240,6 @@ Config_setitem(Config *self, PyObject *py_key, PyObject *py_value)
     return 0;
 }
 
-int
-Config_foreach_callback_wrapper(const git_config_entry *entry, void *c_payload)
-{
-    PyObject *args = (PyObject *)c_payload;
-    PyObject *py_callback = NULL;
-    PyObject *py_payload = NULL;
-    PyObject *py_result = NULL;
-    int c_result;
-
-    if (!PyArg_ParseTuple(args, "O|O", &py_callback, &py_payload))
-        return -1;
-
-    if (py_payload)
-        args = Py_BuildValue("ssO", entry->name, entry->value, py_payload);
-    else
-        args = Py_BuildValue("ss", entry->name, entry->value);
-    if (!args)
-        return -1;
-
-    if (!(py_result = PyObject_CallObject(py_callback, args)))
-        return -1;
-
-    if ((c_result = PyLong_AsLong(py_result)) == -1)
-        return -1;
-
-    Py_CLEAR(args);
-
-    return c_result;
-}
-
-
-PyDoc_STRVAR(Config_foreach__doc__,
-  "foreach(callback[, payload]) -> int\n"
-  "\n"
-  "Perform an operation on each config variable.\n"
-  "\n"
-  "The callback must be of type Callable and receives the normalized name\n"
-  "and value of each variable in the config backend, and an optional payload\n"
-  "passed to this method. As soon as one of the callbacks returns an integer\n"
-  "other than 0, this function returns that value.");
-
-PyObject *
-Config_foreach(Config *self, PyObject *args)
-{
-    int ret;
-    PyObject *py_callback;
-    PyObject *py_payload = NULL;
-
-    if (!PyArg_ParseTuple(args, "O|O", &py_callback, &py_payload))
-        return NULL;
-
-    if (!PyCallable_Check(py_callback)) {
-        PyErr_SetString(PyExc_TypeError,
-                        "Argument 'callback' is not callable");
-        return NULL;
-    }
-
-    ret = git_config_foreach(self->config, Config_foreach_callback_wrapper,
-            (void *)args);
-
-    return PyLong_FromLong((long)ret);
-}
-
-
 PyDoc_STRVAR(Config_add_file__doc__,
   "add_file(path, level=0, force=0)\n"
   "\n"
@@ -407,10 +344,28 @@ Config_set_multivar(Config *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
+PyObject *
+Config_iter(Config *self)
+{
+    ConfigIter *iter;
+    int err;
+
+    iter = PyObject_New(ConfigIter, &ConfigIterType);
+    if (!iter)
+        return NULL;
+
+    if ((err = git_config_iterator_new(&iter->iter, self->config)) < 0)
+        return Error_set(err);
+
+    Py_INCREF(self);
+    iter->owner = self;
+
+    return (PyObject*)iter;
+}
+
 PyMethodDef Config_methods[] = {
     METHOD(Config, get_system_config, METH_NOARGS | METH_STATIC),
     METHOD(Config, get_global_config, METH_NOARGS | METH_STATIC),
-    METHOD(Config, foreach, METH_VARARGS),
     METHOD(Config, add_file, METH_VARARGS | METH_KEYWORDS),
     METHOD(Config, get_multivar, METH_VARARGS),
     METHOD(Config, set_multivar, METH_VARARGS),
@@ -463,7 +418,7 @@ PyTypeObject ConfigType = {
     0,                                         /* tp_clear          */
     0,                                         /* tp_richcompare    */
     0,                                         /* tp_weaklistoffset */
-    0,                                         /* tp_iter           */
+    (getiterfunc)Config_iter,                  /* tp_iter           */
     0,                                         /* tp_iternext       */
     Config_methods,                            /* tp_methods        */
     0,                                         /* tp_members        */
@@ -477,3 +432,56 @@ PyTypeObject ConfigType = {
     0,                                         /* tp_alloc          */
     0,                                         /* tp_new            */
 };
+
+void
+ConfigIter_dealloc(ConfigIter *self)
+{
+    Py_CLEAR(self->owner);
+    git_config_iterator_free(self->iter);
+    PyObject_Del(self);
+}
+
+PyObject *
+ConfigIter_iternext(ConfigIter *self)
+{
+    int err;
+    git_config_entry *entry;
+
+    if ((err = git_config_next(&entry, self->iter)) < 0)
+        return Error_set(err);
+
+    return Py_BuildValue("ss", entry->name, entry->value);
+}
+
+PyDoc_STRVAR(ConfigIter__doc__, "Configuration iterator.");
+
+PyTypeObject ConfigIterType = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "_pygit2.ConfigIter",                       /* tp_name           */
+    sizeof(ConfigIter),                         /* tp_basicsize      */
+    0,                                         /* tp_itemsize       */
+    (destructor)ConfigIter_dealloc ,           /* tp_dealloc        */
+    0,                                         /* tp_print          */
+    0,                                         /* tp_getattr        */
+    0,                                         /* tp_setattr        */
+    0,                                         /* tp_compare        */
+    0,                                         /* tp_repr           */
+    0,                                         /* tp_as_number      */
+    0,                                         /* tp_as_sequence    */
+    0,                                         /* tp_as_mapping     */
+    0,                                         /* tp_hash           */
+    0,                                         /* tp_call           */
+    0,                                         /* tp_str            */
+    0,                                         /* tp_getattro       */
+    0,                                         /* tp_setattro       */
+    0,                                         /* tp_as_buffer      */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /* tp_flags          */
+    ConfigIter__doc__,                         /* tp_doc            */
+    0,                                         /* tp_traverse       */
+    0,                                         /* tp_clear          */
+    0,                                         /* tp_richcompare    */
+    0,                                         /* tp_weaklistoffset */
+    PyObject_SelfIter,                         /* tp_iter           */
+    (iternextfunc)ConfigIter_iternext,         /* tp_iternext       */
+
+};

src/pygit2.c

@@ -56,6 +56,7 @@ extern PyTypeObject IndexEntryType;
 extern PyTypeObject IndexIterType;
 extern PyTypeObject WalkerType;
 extern PyTypeObject ConfigType;
+extern PyTypeObject ConfigIterType;
 extern PyTypeObject ReferenceType;
 extern PyTypeObject RefLogIterType;
 extern PyTypeObject RefLogEntryType;
@@ -412,7 +413,9 @@ moduleinit(PyObject* m)
 
     /* Config */
     INIT_TYPE(ConfigType, NULL, PyType_GenericNew)
+    INIT_TYPE(ConfigIterType, NULL, PyType_GenericNew)
     ADD_TYPE(m, Config)
+    ADD_TYPE(m, ConfigIter)
 
     /* Remotes */
     INIT_TYPE(RemoteType, NULL, NULL)

src/types.h

@@ -77,6 +77,11 @@ typedef struct {
     git_config* config;
 } Config;
 
+typedef struct {
+    PyObject_HEAD
+    Config *owner;
+    git_config_iterator *iter;
+} ConfigIter;
 
 /* git_note */
 typedef struct {

test/test_config.py

@@ -36,13 +36,6 @@ from . import utils
 
 CONFIG_FILENAME = "test_config"
 
-
-def foreach_test_wrapper(key, name, lst):
-    lst[key] = name
-    return 0
-foreach_test_wrapper.__test__ = False
-
-
 class ConfigTest(utils.RepoTestCase):
 
     def tearDown(self):
@@ -175,13 +168,15 @@ class ConfigTest(utils.RepoTestCase):
         for i in l:
             self.assertEqual(i, 'foo-123456')
 
-    def test_foreach(self):
+    def test_iterator(self):
         config = self.repo.config
         lst = {}
-        config.foreach(foreach_test_wrapper, lst)
+
+        for name, value in config:
+            lst[name] = value
+
         self.assertTrue('core.bare' in lst)
         self.assertTrue(lst['core.bare'])
 
-
 if __name__ == '__main__':
     unittest.main()