refs: improve API (#213)

Changes:

- Reference.oid and Reference.hex removed
- Now Reference.target can be assigned an oid

docs/references.rst

@@ -28,8 +28,6 @@ The Reference type
 ====================
 
 .. autoattribute:: pygit2.Reference.name
-.. autoattribute:: pygit2.Reference.oid
-.. autoattribute:: pygit2.Reference.hex
 .. autoattribute:: pygit2.Reference.target
 .. autoattribute:: pygit2.Reference.type
 

src/reference.c

@@ -214,37 +214,55 @@ Reference_target__get__(Reference *self)
         PyErr_SetString(PyExc_ValueError, "no target available");
         return NULL;
     }
-
     return to_path(c_name);
 }
 
 int
-Reference_target__set__(Reference *self, PyObject *py_name)
+Reference_target__set__(Reference *self, PyObject *py_target)
 {
+    git_oid oid;
     char *c_name;
     int err;
     git_reference *new_ref;
+    git_repository *repo;
 
     CHECK_REFERENCE_INT(self);
 
-    /* Get the C name */
+    /* Case 1: Direct */
-    c_name = py_path_to_c_str(py_name);
+    if (GIT_REF_OID == git_reference_type(self->reference)) {
-    if (c_name == NULL)
+        repo = git_reference_owner(self->reference);
-        return -1;
+        err = py_str_to_git_oid_expand(repo, py_target, &oid);
+        if (err < 0)
+            goto error;
 
-    /* Set the new target */
+        err = git_reference_set_target(&new_ref, self->reference, &oid);
-    err = git_reference_symbolic_set_target(&new_ref, self->reference, c_name);
+        if (err < 0)
-    free(c_name);
+            goto error;
-    if (err < 0) {
-        Error_set(err);
-        return -1;
-    }
 
         git_reference_free(self->reference);
         self->reference = new_ref;
         return 0;
     }
 
+    /* Case 2: Symbolic */
+    c_name = py_path_to_c_str(py_target);
+    if (c_name == NULL)
+        return -1;
+
+    err = git_reference_symbolic_set_target(&new_ref, self->reference, c_name);
+    free(c_name);
+    if (err < 0)
+        goto error;
+
+    git_reference_free(self->reference);
+    self->reference = new_ref;
+    return 0;
+
+error:
+    Error_set(err);
+    return -1;
+}
+
 
 PyDoc_STRVAR(Reference_name__doc__, "The full name of a reference.");
 
@@ -256,77 +274,6 @@ Reference_name__get__(Reference *self)
 }
 
 
-PyDoc_STRVAR(Reference_oid__doc__, "Object id.");
-
-PyObject *
-Reference_oid__get__(Reference *self)
-{
-    CHECK_REFERENCE(self);
-
-    /* Case 1: Direct */
-    if (GIT_REF_OID == git_reference_type(self->reference))
-        return git_oid_to_python(git_reference_target(self->reference));
-
-    /* Get the oid (only for "direct" references) */
-    PyErr_SetString(PyExc_ValueError,
-                    "oid is only available if the reference is direct "
-                    "(i.e. not symbolic)");
-    return NULL;
-}
-
-int
-Reference_oid__set__(Reference *self, PyObject *py_hex)
-{
-    git_oid oid;
-    int err;
-    git_reference *new_ref;
-
-    CHECK_REFERENCE_INT(self);
-
-    /* Get the oid */
-    err = py_str_to_git_oid_expand(git_reference_owner(self->reference),
-                                                       py_hex, &oid);
-    if (err < 0) {
-        Error_set(err);
-        return -1;
-    }
-
-    /* Set the oid */
-    err = git_reference_set_target(&new_ref, self->reference, &oid);
-    if (err < 0) {
-        Error_set(err);
-        return -1;
-    }
-
-    git_reference_free(self->reference);
-    self->reference = new_ref;
-    return 0;
-}
-
-
-PyDoc_STRVAR(Reference_hex__doc__, "Hex oid.");
-
-PyObject *
-Reference_hex__get__(Reference *self)
-{
-    const git_oid *oid;
-
-    CHECK_REFERENCE(self);
-
-    /* Get the oid (only for "direct" references) */
-    oid = git_reference_target(self->reference);
-    if (oid == NULL) {
-        PyErr_SetString(PyExc_ValueError,
-                        "oid is only available if the reference is direct "
-                        "(i.e. not symbolic)");
-        return NULL;
-    }
-
-    /* Convert and return it */
-    return git_oid_to_py_str(oid);
-}
-
-
 PyDoc_STRVAR(Reference_type__doc__,
   "Type (GIT_REF_OID or GIT_REF_SYMBOLIC).");
 
@@ -460,8 +407,6 @@ PyMethodDef Reference_methods[] = {
 
 PyGetSetDef Reference_getseters[] = {
     GETTER(Reference, name),
-    GETSET(Reference, oid),
-    GETTER(Reference, hex),
     GETSET(Reference, target),
     GETTER(Reference, type),
     {NULL}

test/test_diff.py

@@ -91,7 +91,7 @@ a contents
 class DiffDirtyTest(utils.DirtyRepoTestCase):
     def test_diff_empty_index(self):
         repo = self.repo
-        head = repo[repo.lookup_reference('HEAD').resolve().oid]
+        head = repo[repo.lookup_reference('HEAD').resolve().target]
         diff = head.tree.diff(repo.index)
 
         files = [patch.new_file_path for patch in diff]
@@ -99,12 +99,13 @@ class DiffDirtyTest(utils.DirtyRepoTestCase):
 
     def test_workdir_to_tree(self):
         repo = self.repo
-        head = repo[repo.lookup_reference('HEAD').resolve().oid]
+        head = repo[repo.lookup_reference('HEAD').resolve().target]
         diff = head.tree.diff()
 
         files = [patch.new_file_path for patch in diff]
         self.assertEqual(DIFF_WORKDIR_EXPECTED, files)
 
+
 class DiffTest(utils.BareRepoTestCase):
 
     def test_diff_invalid(self):
@@ -114,7 +115,7 @@ class DiffTest(utils.BareRepoTestCase):
 
     def test_diff_empty_index(self):
         repo = self.repo
-        head = repo[repo.lookup_reference('HEAD').resolve().oid]
+        head = repo[repo.lookup_reference('HEAD').resolve().target]
         diff = head.tree.diff(repo.index)
 
         files = [patch.new_file_path.split('/')[0] for patch in diff]

test/test_note.py

@@ -56,16 +56,17 @@ class NotesTest(utils.BareRepoTestCase):
         self.assertEqual(NOTE[1].encode(), self.repo[note_id].data)
 
     def test_lookup_note(self):
-        annotated_id = self.repo.head.hex
+        annotated_id = self.repo.head.target.hex
         note = self.repo.lookup_note(annotated_id)
         self.assertEqual(NOTES[0][0], note.oid.hex)
         self.assertEqual(NOTES[0][1], note.message)
 
     def test_remove_note(self):
-        note = self.repo.lookup_note(self.repo.head.hex)
+        head = self.repo.head
+        note = self.repo.lookup_note(head.target.hex)
         author = committer = Signature('Foo bar', 'foo@bar.com', 12346, 0)
         note.remove(author, committer)
-        self.assertRaises(KeyError, self.repo.lookup_note, self.repo.head.hex)
+        self.assertRaises(KeyError, self.repo.lookup_note, head.target.hex)
 
     def test_iterate_notes(self):
         for i, note in enumerate(self.repo.notes()):

test/test_refs.py

@@ -61,7 +61,7 @@ class ReferencesTest(utils.RepoTestCase):
 
     def test_head(self):
         head = self.repo.head
-        self.assertEqual(LAST_COMMIT, self.repo[head.oid].hex)
+        self.assertEqual(LAST_COMMIT, self.repo[head.target].hex)
 
     def test_lookup_reference(self):
         repo = self.repo
@@ -76,20 +76,20 @@ class ReferencesTest(utils.RepoTestCase):
 
     def test_reference_get_sha(self):
         reference = self.repo.lookup_reference('refs/heads/master')
-        self.assertEqual(reference.hex, LAST_COMMIT)
+        self.assertEqual(reference.target.hex, LAST_COMMIT)
 
 
     def test_reference_set_sha(self):
         NEW_COMMIT = '5ebeeebb320790caf276b9fc8b24546d63316533'
         reference = self.repo.lookup_reference('refs/heads/master')
-        reference.oid = NEW_COMMIT
+        reference.target = NEW_COMMIT
-        self.assertEqual(reference.hex, NEW_COMMIT)
+        self.assertEqual(reference.target.hex, NEW_COMMIT)
 
     def test_reference_set_sha_prefix(self):
         NEW_COMMIT = '5ebeeebb320790caf276b9fc8b24546d63316533'
         reference = self.repo.lookup_reference('refs/heads/master')
-        reference.oid = NEW_COMMIT[0:6]
+        reference.target = NEW_COMMIT[0:6]
-        self.assertEqual(reference.hex, NEW_COMMIT)
+        self.assertEqual(reference.target.hex, NEW_COMMIT)
 
 
     def test_reference_get_type(self):
@@ -123,10 +123,8 @@ class ReferencesTest(utils.RepoTestCase):
         # Access the deleted reference
         self.assertRaises(GitError, getattr, reference, 'name')
         self.assertRaises(GitError, getattr, reference, 'type')
-        self.assertRaises(GitError, getattr, reference, 'oid')
-        self.assertRaises(GitError, setattr, reference, 'oid', LAST_COMMIT)
-        self.assertRaises(GitError, getattr, reference, 'hex')
         self.assertRaises(GitError, getattr, reference, 'target')
+        self.assertRaises(GitError, setattr, reference, 'target', LAST_COMMIT)
         self.assertRaises(GitError, setattr, reference, 'target', "a/b/c")
         self.assertRaises(GitError, reference.delete)
         self.assertRaises(GitError, reference.resolve)
@@ -159,7 +157,7 @@ class ReferencesTest(utils.RepoTestCase):
         self.assertEqual(reference.type, GIT_REF_SYMBOLIC)
         reference = reference.resolve()
         self.assertEqual(reference.type, GIT_REF_OID)
-        self.assertEqual(reference.hex, LAST_COMMIT)
+        self.assertEqual(reference.target.hex, LAST_COMMIT)
 
 
     def test_reference_resolve_identity(self):
@@ -175,7 +173,6 @@ class ReferencesTest(utils.RepoTestCase):
         refs = self.repo.listall_references()
         self.assertTrue('refs/tags/version1' in refs)
         reference = self.repo.lookup_reference('refs/tags/version1')
-        self.assertEqual(reference.hex, LAST_COMMIT)
         self.assertEqual(reference.target.hex, LAST_COMMIT)
 
         # try to create existing reference
@@ -185,7 +182,7 @@ class ReferencesTest(utils.RepoTestCase):
         # try to create existing reference with force
         reference =  self.repo.create_reference('refs/tags/version1',
                         LAST_COMMIT, force=True)
-        self.assertEqual(reference.hex, LAST_COMMIT)
+        self.assertEqual(reference.target.hex, LAST_COMMIT)
 
 
     def test_create_symbolic_reference(self):

test/test_repository.py

@@ -63,7 +63,7 @@ class RepositoryTest(utils.BareRepoTestCase):
 
     def test_head(self):
         head = self.repo.head
-        self.assertEqual(HEAD_SHA, head.hex)
+        self.assertEqual(HEAD_SHA, head.target.hex)
         self.assertEqual(type(head), Reference)
         self.assertFalse(self.repo.head_is_orphaned)
         self.assertFalse(self.repo.head_is_detached)