From bde58d972742707094eeb16ac7b1d9dfdf75f342 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Sun, 8 Jun 2014 20:35:21 +0200
Subject: [PATCH] Remote: make renaming take a method call

Renaming a remote in pygit2 has been done via Remote.name= up to now,
but this is inherently unsafe, as it provides no way to pass up the
refspecs that libgit2 was unable to remap.

In fact, if there ever was such problem, we would have segfaulted.

libgit2 now provides a much more direct way of getting back the results,
so expose it as the return value of Remote.rename(). This also removes
the hint that a rename might be something that happens only to the
in-memory structure.
---
 pygit2/decl.h       | 10 +++++-----
 pygit2/remote.py    | 19 +++++++++++++++----
 test/test_remote.py |  9 +++++----
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/pygit2/decl.h b/pygit2/decl.h
index 862f867..d3b5b02 100644
--- a/pygit2/decl.h
+++ b/pygit2/decl.h
@@ -116,11 +116,11 @@ int git_remote_create(
 	const char *url);
 
 const char * git_remote_name(const git_remote *remote);
-typedef int (*git_remote_rename_problem_cb)(const char *problematic_refspec, void *payload);
-int git_remote_rename(git_remote *remote,
-	const char *new_name,
-	git_remote_rename_problem_cb callback,
-	void *payload);
+
+int git_remote_rename(
+	git_strarray *problems,
+	git_remote *remote,
+	const char *new_name);
 const char * git_remote_url(const git_remote *remote);
 int git_remote_set_url(git_remote *remote, const char* url);
 const char * git_remote_pushurl(const git_remote *remote);
diff --git a/pygit2/remote.py b/pygit2/remote.py
index 890e074..2b7f32d 100644
--- a/pygit2/remote.py
+++ b/pygit2/remote.py
@@ -142,14 +142,25 @@ class Remote(object):
 
         return maybe_string(C.git_remote_name(self._remote))
 
-    @name.setter
-    def name(self, value):
-        if not value:
+    def rename(self, new_name):
+        """Rename this remote
+
+        Returns a list of fetch refspecs which were not in the standard format
+        and thus could not be remapped
+        """
+
+        if not new_name:
             raise ValueError("New remote name must be a non-empty string")
 
-        err = C.git_remote_rename(self._remote, to_str(value), ffi.NULL, ffi.NULL)
+        problems = ffi.new('git_strarray *')
+        err = C.git_remote_rename(problems, self._remote, to_str(new_name))
         check_error(err)
 
+        ret = strarray_to_strings(problems)
+        C.git_strarray_free(problems)
+
+        return ret
+
     @property
     def url(self):
         """Url of the remote"""
diff --git a/test/test_remote.py b/test/test_remote.py
index 9c95f80..57814a3 100644
--- a/test/test_remote.py
+++ b/test/test_remote.py
@@ -61,11 +61,12 @@ class RepositoryTest(utils.RepoTestCase):
         remote = self.repo.remotes[0]
 
         self.assertEqual(REMOTE_NAME, remote.name)
-        remote.name = 'new'
+        problems = remote.rename('new')
+        self.assertEqual([], problems)
         self.assertEqual('new', remote.name)
 
-        self.assertRaisesAssign(ValueError, remote, 'name', '')
-        self.assertRaisesAssign(ValueError, remote, 'name', None)
+        self.assertRaises(ValueError, remote.rename, '')
+        self.assertRaises(ValueError, remote.rename, None)
 
 
     def test_remote_set_url(self):
@@ -153,7 +154,7 @@ class RepositoryTest(utils.RepoTestCase):
     def test_remote_save(self):
         remote = self.repo.remotes[0]
 
-        remote.name = 'new-name'
+        remote.rename('new-name')
         remote.url = 'http://example.com/test.git'
 
         remote.save()