Remote: make fetch/push_refspecs attributes

This is a lot more pythonic than two sets of getter-setter
functions. The old ones are left for backwards compatibility but they
should be removed in the next release.

src/remote.c

@@ -370,49 +370,6 @@ get_pylist_from_git_strarray(git_strarray *strarray)
     return new_list;
 }
 
-
-PyDoc_STRVAR(Remote_get_fetch_refspecs__doc__, "Fetch refspecs");
-
-
-PyObject *
-Remote_get_fetch_refspecs(Remote *self)
-{
-    int err;
-    git_strarray refspecs;
-    PyObject *new_list;
-
-    err = git_remote_get_fetch_refspecs(&refspecs, self->remote);
-    if (err != GIT_OK)
-        return Error_set(err);
-
-    new_list = get_pylist_from_git_strarray(&refspecs);
-
-    git_strarray_free(&refspecs);
-    return new_list;
-}
-
-
-PyDoc_STRVAR(Remote_get_push_refspecs__doc__, "Push refspecs");
-
-
-PyObject *
-Remote_get_push_refspecs(Remote *self)
-{
-    int err;
-    git_strarray refspecs;
-    PyObject *new_list;
-
-    err = git_remote_get_push_refspecs(&refspecs, self->remote);
-    if (err != GIT_OK)
-        return Error_set(err);
-
-    new_list = get_pylist_from_git_strarray(&refspecs);
-
-    git_strarray_free(&refspecs);
-    return new_list;
-}
-
-
 int
 get_strarraygit_from_pylist(git_strarray *array, PyObject *pylist)
 {
@@ -442,59 +399,142 @@ get_strarraygit_from_pylist(git_strarray *array, PyObject *pylist)
     return GIT_OK;
 }
 
-
+PyDoc_STRVAR(Remote_fetch_refspecs__doc__, "Fetch refspecs");
-PyDoc_STRVAR(Remote_set_fetch_refspecs__doc__,
-    "set_fetch_refspecs([str])\n"
-    "\n");
-
 
 PyObject *
-Remote_set_fetch_refspecs(Remote *self, PyObject *args)
+Remote_fetch_refspecs__get__(Remote *self)
+{
+    int err;
+    git_strarray refspecs;
+    PyObject *new_list;
+
+    err = git_remote_get_fetch_refspecs(&refspecs, self->remote);
+    if (err != GIT_OK)
+        return Error_set(err);
+
+    new_list = get_pylist_from_git_strarray(&refspecs);
+
+    git_strarray_free(&refspecs);
+    return new_list;
+}
+
+int
+Remote_fetch_refspecs__set__(Remote *self, PyObject *args)
 {
     int err;
     PyObject *pyrefspecs;
     git_strarray fetch_refspecs;
 
     if (! PyArg_Parse(args, "O", &pyrefspecs))
-        return NULL;
+        return -1;
 
-    if (get_strarraygit_from_pylist(&fetch_refspecs, pyrefspecs) != GIT_OK)
+    if (get_strarraygit_from_pylist(&fetch_refspecs, pyrefspecs) < 0)
-        return NULL;
+        return -1;
 
     err = git_remote_set_fetch_refspecs(self->remote, &fetch_refspecs);
     git_strarray_free(&fetch_refspecs);
 
+    if (err < 0) {
+        Error_set(err);
+        return -1;
+    }
+
+    return 0;
+}
+
+PyDoc_STRVAR(Remote_push_refspecs__doc__, "Push refspecs");
+
+PyObject *
+Remote_push_refspecs__get__(Remote *self)
+{
+    int err;
+    git_strarray refspecs;
+    PyObject *new_list;
+
+    err = git_remote_get_push_refspecs(&refspecs, self->remote);
     if (err != GIT_OK)
         return Error_set(err);
 
+    new_list = get_pylist_from_git_strarray(&refspecs);
+
+    git_strarray_free(&refspecs);
+    return new_list;
+}
+
+int
+Remote_push_refspecs__set__(Remote *self, PyObject *args)
+{
+    int err;
+    PyObject *pyrefspecs;
+    git_strarray push_refspecs;
+
+    if (! PyArg_Parse(args, "O", &pyrefspecs))
+        return -1;
+
+    if (get_strarraygit_from_pylist(&push_refspecs, pyrefspecs) != 0)
+        return -1;
+
+    err = git_remote_set_push_refspecs(self->remote, &push_refspecs);
+    git_strarray_free(&push_refspecs);
+
+    if (err < 0) {
+        Error_set(err);
+        return -1;
+    }
+
+    return 0;
+}
+
+PyDoc_STRVAR(Remote_get_fetch_refspecs__doc__,
+    "Fetch refspecs.\n"
+    "This function is deprecated, please use the fetch_refspecs attribute"
+    "\n");
+
+
+PyObject *
+Remote_get_fetch_refspecs(Remote *self)
+{
+    return Remote_fetch_refspecs__get__(self);
+}
+
+
+PyDoc_STRVAR(Remote_get_push_refspecs__doc__, "Push refspecs");
+
+
+PyObject *
+Remote_get_push_refspecs(Remote *self)
+{
+    return Remote_push_refspecs__get__(self);
+}
+
+PyDoc_STRVAR(Remote_set_fetch_refspecs__doc__,
+    "set_fetch_refspecs([str])\n"
+    "This function is deprecated, please use the push_refspecs attribute"
+    "\n");
+
+
+PyObject *
+Remote_set_fetch_refspecs(Remote *self, PyObject *args)
+{
+    if (Remote_fetch_refspecs__set__(self, args) < 0)
+        return NULL;
+
     Py_RETURN_NONE;
 }
 
 
 PyDoc_STRVAR(Remote_set_push_refspecs__doc__,
     "set_push_refspecs([str])\n"
+    "This function is deprecated, please use the push_refspecs attribute"
     "\n");
 
 
 PyObject *
 Remote_set_push_refspecs(Remote *self, PyObject *args)
 {
-    int err;
+    if (Remote_push_refspecs__set__(self, args) < 0)
-    PyObject *pyrefspecs;
-    git_strarray push_refspecs;
-
-    if (! PyArg_Parse(args, "O", &pyrefspecs))
         return NULL;
 
-    if (get_strarraygit_from_pylist(&push_refspecs, pyrefspecs) != 0)
-        return NULL;
-
-    err = git_remote_set_push_refspecs(self->remote, &push_refspecs);
-    git_strarray_free(&push_refspecs);
-
-    if (err != GIT_OK)
-        return Error_set(err);
-
     Py_RETURN_NONE;
 }
 
@@ -795,6 +835,8 @@ PyGetSetDef Remote_getseters[] = {
     GETSET(Remote, url),
     GETSET(Remote, push_url),
     GETTER(Remote, refspec_count),
+    GETSET(Remote, fetch_refspecs),
+    GETSET(Remote, push_refspecs),
     {NULL}
 };
 

test/test_remote.py

@@ -103,10 +103,19 @@ class RepositoryTest(utils.RepoTestCase):
         self.assertEqual(list, type(remote.get_push_refspecs()))
         self.assertEqual(0, len(remote.get_push_refspecs()))
 
+        push_specs = remote.push_refspecs
+        self.assertEqual(list, type(push_specs))
+        self.assertEqual(0, len(push_specs))
+
         remote.set_fetch_refspecs(['+refs/*:refs/remotes/*'])
         self.assertEqual('+refs/*:refs/remotes/*',
                          remote.get_fetch_refspecs()[0])
 
+        fetch_specs = remote.fetch_refspecs
+        self.assertEqual(list, type(fetch_specs))
+        self.assertEqual(1, len(fetch_specs))
+        self.assertEqual('+refs/*:refs/remotes/*', fetch_specs[0])
+
         remote.set_fetch_refspecs([
            '+refs/*:refs/remotes/*',
            '+refs/test/*:refs/test/remotes/*'