From 204fbd9cbb93bf1210b5233993c24626a55f5981 Mon Sep 17 00:00:00 2001
From: Nico von Geyso <Nico.Geyso@FU-Berlin.de>
Date: Mon, 4 Mar 2013 15:45:17 +0100
Subject: [PATCH] diff - use generator instead of list
---
include/pygit2/types.h | 14 ++-
src/diff.c | 253 +++++++++++++++++++++++++++++++----------
src/index.c | 2 +-
src/pygit2.c | 6 +
src/tree.c | 3 +-
test/test_diff.py | 39 +++----
6 files changed, 230 insertions(+), 87 deletions(-)
diff --git a/include/pygit2/types.h b/include/pygit2/types.h
index 7625b89..dfd08c0 100644
--- a/include/pygit2/types.h
+++ b/include/pygit2/types.h
@@ -59,14 +59,20 @@ OBJECT_STRUCT(Index, git_index, index)
OBJECT_STRUCT(Walker, git_revwalk, walk)
OBJECT_STRUCT(Config, git_config, config)
OBJECT_STRUCT(Remote, git_remote, remote)
+OBJECT_STRUCT(Diff, git_diff_list, list)
typedef struct {
PyObject_HEAD
- Repository *repo;
- git_diff_list *diff;
- PyObject *diff_changes;
-} Diff;
+ git_diff_list* list;
+ size_t i;
+ size_t n;
+} DiffIter;
+typedef struct {
+ PyObject_HEAD
+ PyObject* files;
+ PyObject* hunks;
+} DiffEntry;
typedef struct {
PyObject_HEAD
diff --git a/src/diff.c b/src/diff.c
index de19fdd..c9b4983 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -40,35 +40,29 @@ extern PyTypeObject IndexType;
extern PyTypeObject DiffType;
extern PyTypeObject HunkType;
+PyTypeObject DiffEntryType;
-PyDoc_STRVAR(Diff_changes__doc__, "Raw changes.");
-
-PyObject *
-Diff_changes__get__(Diff *self)
+PyObject*
+diff_get_patch_byindex(git_diff_list* list, size_t i)
{
const git_diff_delta* delta;
const git_diff_range* range;
- git_diff_patch* patch;
+ git_diff_patch* patch = NULL;
+
char buffer[41];
const char* hunk_content;
- size_t amounts, hunk_amounts, i, j, hunk_header_len, hunk_lines;
- PyObject *file, *files, *hunks;
- Hunk *py_hunk;
+ size_t hunk_amounts, j, hunk_header_len, hunk_lines;
int err;
- if (self->diff_changes == NULL) {
- self->diff_changes = PyDict_New();
+ PyObject *file;
+ Hunk *py_hunk;
+ DiffEntry *py_entry = NULL;
- files = PyList_New(0);
- PyDict_SetItemString(self->diff_changes, "files", files);
-
- hunks = PyList_New(0);
- PyDict_SetItemString(self->diff_changes, "hunks", hunks);
-
- amounts = git_diff_num_deltas(self->diff);
- for (i = 0; i < amounts ; ++i) {
- err = git_diff_get_patch(&patch, &delta, self->diff, i);
+ err = git_diff_get_patch(&patch, &delta, list, i);
+ if (err == GIT_OK) {
+ py_entry = (DiffEntry*) INSTANCIATE_CLASS(DiffEntryType, NULL);
+ if (py_entry != NULL) {
if (err == GIT_OK) {
file = Py_BuildValue("(s,s,i,i)",
delta->old_file.path,
@@ -77,7 +71,7 @@ Diff_changes__get__(Diff *self)
delta->similarity
);
- PyList_Append(files, file);
+ PyList_Append((PyObject*) py_entry->files, file);
}
hunk_amounts = git_diff_patch_num_hunks(patch);
@@ -108,16 +102,147 @@ Diff_changes__get__(Diff *self)
py_hunk->data = Py_BuildValue("(s#,i)",
hunk_content, hunk_header_len,
hunk_lines);
- PyList_Append(hunks, (PyObject*) py_hunk);
+
+ PyList_Append((PyObject*) py_entry->hunks,
+ (PyObject*) py_hunk);
}
}
}
}
}
- return PyDict_Copy(self->diff_changes);
+ if (err < 0)
+ return Error_set(err);
+
+ return (PyObject*) py_entry;
}
+PyObject *
+DiffEntry_call(DiffEntry *self, PyObject *args, PyObject *kwds)
+{
+ self->files = PyList_New(0);
+ if (self->files == NULL) {
+ Py_XDECREF(self);
+ return NULL;
+ }
+
+ self->hunks = PyList_New(0);
+ if (self->hunks == NULL) {
+ Py_XDECREF(self);
+ return NULL;
+ }
+
+ return (PyObject*) self;
+}
+
+static void
+DiffEntry_dealloc(DiffEntry *self)
+{
+ Py_DECREF((PyObject*) self->files);
+ Py_DECREF((PyObject*) self->hunks);
+ PyObject_Del(self);
+}
+
+PyMemberDef DiffEntry_members[] = {
+ MEMBER(DiffEntry, files, T_OBJECT, "files"),
+ MEMBER(DiffEntry, hunks, T_OBJECT, "hunks"),
+ {NULL}
+};
+
+PyDoc_STRVAR(DiffEntry__doc__, "Diff entry object.");
+
+PyTypeObject DiffEntryType = {
+ PyVarObject_HEAD_INIT(NULL, 0)
+ "_pygit2.DiffEntry", /* tp_name */
+ sizeof(DiffEntry), /* tp_basicsize */
+ 0, /* tp_itemsize */
+ (destructor)DiffEntry_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 */
+ (ternaryfunc) DiffEntry_call, /* tp_call */
+ 0, /* tp_str */
+ 0, /* tp_getattro */
+ 0, /* tp_setattro */
+ 0, /* tp_as_buffer */
+ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
+ DiffEntry__doc__, /* tp_doc */
+ 0, /* tp_traverse */
+ 0, /* tp_clear */
+ 0, /* tp_richcompare */
+ 0, /* tp_weaklistoffset */
+ 0, /* tp_iter */
+ 0, /* tp_iternext */
+ 0, /* tp_methods */
+ DiffEntry_members, /* tp_members */
+ 0, /* tp_getset */
+ 0, /* tp_base */
+ 0, /* tp_dict */
+ 0, /* tp_descr_get */
+ 0, /* tp_descr_set */
+ 0, /* tp_dictoffset */
+ 0, /* tp_init */
+ 0, /* tp_alloc */
+ 0, /* tp_new */
+};
+
+
+PyObject *
+DiffIter_iternext(DiffIter *self)
+{
+ if (self->i < self->n)
+ return diff_get_patch_byindex(self->list, self->i++);
+
+ PyErr_SetNone(PyExc_StopIteration);
+ return NULL;
+}
+
+void
+DiffIter_dealloc(DiffIter *self)
+{
+ Py_CLEAR(self->list);
+ PyObject_Del(self);
+}
+
+
+PyDoc_STRVAR(DiffIter__doc__, "Diff iterator object.");
+
+PyTypeObject DiffIterType = {
+ PyVarObject_HEAD_INIT(NULL, 0)
+ "_pygit2.DiffIter", /* tp_name */
+ sizeof(DiffIter), /* tp_basicsize */
+ 0, /* tp_itemsize */
+ (destructor)DiffIter_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 */
+ DiffIter__doc__, /* tp_doc */
+ 0, /* tp_traverse */
+ 0, /* tp_clear */
+ 0, /* tp_richcompare */
+ 0, /* tp_weaklistoffset */
+ PyObject_SelfIter, /* tp_iter */
+ (iternextfunc) DiffIter_iternext, /* tp_iternext */
+};
+
PyDoc_STRVAR(Diff_patch__doc__, "Patch.");
@@ -129,11 +254,11 @@ Diff_patch__get__(Diff *self)
char* str = NULL, *buffer = NULL;
int err = 0;
size_t i, len, num, size;
- PyObject *py_patch;
+ PyObject *py_patch = NULL;
- num = git_diff_num_deltas(self->diff);
+ num = git_diff_num_deltas(self->list);
for (i = 0; i < num ; ++i) {
- err = git_diff_get_patch(&patch, &delta, self->diff, i);
+ err = git_diff_get_patch(&patch, &delta, self->list, i);
if (err < 0 || (err = git_diff_patch_to_str(&str, patch)) < 0)
goto error;
@@ -160,30 +285,6 @@ error:
return (err < 0) ? Error_set(err) : py_patch;
}
-static int
-Hunk_init(Hunk *self, PyObject *args, PyObject *kwds)
-{
- self->header = NULL;
-
- self->old_file = NULL;
- self->old_start = 0;
- self->old_lines = 0;
-
- self->new_file = NULL;
- self->new_start = 0;
- self->new_lines = 0;
-
- self->old_oid = NULL;
- self->new_oid = NULL;
-
- self->data = PyList_New(0);
- if (self->data == NULL) {
- Py_XDECREF(self);
- return -1;
- }
-
- return 0;
-}
static void
Hunk_dealloc(Hunk *self)
@@ -258,7 +359,7 @@ PyTypeObject HunkType = {
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
- (initproc)Hunk_init, /* tp_init */
+ 0, /* tp_init */
0, /* tp_alloc */
0, /* tp_new */
};
@@ -277,15 +378,14 @@ Diff_merge(Diff *self, PyObject *args)
if (!PyArg_ParseTuple(args, "O!", &DiffType, &py_diff))
return NULL;
+
if (py_diff->repo->repo != self->repo->repo)
return Error_set(GIT_ERROR);
- err = git_diff_merge(self->diff, py_diff->diff);
+ err = git_diff_merge(self->list, py_diff->list);
if (err < 0)
return Error_set(err);
- Py_XDECREF(self->diff_changes);
- self->diff_changes = NULL;
Py_RETURN_NONE;
}
@@ -304,30 +404,61 @@ Diff_find_similar(Diff *self, PyObject *args)
if (!PyArg_ParseTuple(args, "|i", &opts.flags))
return NULL;
- err = git_diff_find_similar(self->diff, &opts);
+ err = git_diff_find_similar(self->list, &opts);
if (err < 0)
return Error_set(err);
- Py_XDECREF(self->diff_changes);
- self->diff_changes = NULL;
Py_RETURN_NONE;
}
+PyObject *
+Diff_iter(Diff *self)
+{
+ DiffIter *iter;
+
+ iter = PyObject_New(DiffIter, &DiffIterType);
+ if (iter) {
+ Py_INCREF(self);
+ iter->list = self->list;
+ iter->i = 0;
+ iter->n = git_diff_num_deltas(self->list);
+ }
+ return (PyObject*)iter;
+}
+
+PyObject *
+Diff_getitem(Diff *self, PyObject *value)
+{
+ size_t i;
+
+ if (PyLong_Check(value) < 0)
+ return NULL;
+
+ i = PyLong_AsUnsignedLong(value);
+
+ return diff_get_patch_byindex(self->list, i);
+}
+
+
static void
Diff_dealloc(Diff *self)
{
- git_diff_list_free(self->diff);
+ git_diff_list_free(self->list);
Py_XDECREF(self->repo);
- Py_XDECREF(self->diff_changes);
PyObject_Del(self);
}
PyGetSetDef Diff_getseters[] = {
- GETTER(Diff, changes),
GETTER(Diff, patch),
{NULL}
};
+PyMappingMethods Diff_as_mapping = {
+ 0, /* mp_length */
+ (binaryfunc)Diff_getitem, /* mp_subscript */
+ 0, /* mp_ass_subscript */
+};
+
static PyMethodDef Diff_methods[] = {
METHOD(Diff, merge, METH_VARARGS),
METHOD(Diff, find_similar, METH_VARARGS),
@@ -350,7 +481,7 @@ PyTypeObject DiffType = {
0, /* tp_repr */
0, /* tp_as_number */
0, /* tp_as_sequence */
- 0, /* tp_as_mapping */
+ &Diff_as_mapping, /* tp_as_mapping */
0, /* tp_hash */
0, /* tp_call */
0, /* tp_str */
@@ -363,7 +494,7 @@ PyTypeObject DiffType = {
0, /* tp_clear */
0, /* tp_richcompare */
0, /* tp_weaklistoffset */
- 0, /* tp_iter */
+ (getiterfunc)Diff_iter, /* tp_iter */
0, /* tp_iternext */
Diff_methods, /* tp_methods */
0, /* tp_members */
diff --git a/src/index.c b/src/index.c
index dfcb355..79554fc 100644
--- a/src/index.c
+++ b/src/index.c
@@ -158,7 +158,7 @@ Index_diff(Index *self, PyObject *args)
if (py_diff) {
Py_INCREF(self->repo);
py_diff->repo = self->repo;
- py_diff->diff = diff;
+ py_diff->list = diff;
}
return (PyObject*)py_diff;
diff --git a/src/pygit2.c b/src/pygit2.c
index f2acbf0..1c3f42b 100644
--- a/src/pygit2.c
+++ b/src/pygit2.c
@@ -41,6 +41,8 @@ extern PyTypeObject RepositoryType;
extern PyTypeObject ObjectType;
extern PyTypeObject CommitType;
extern PyTypeObject DiffType;
+extern PyTypeObject DiffIterType;
+extern PyTypeObject DiffEntryType;
extern PyTypeObject HunkType;
extern PyTypeObject TreeType;
extern PyTypeObject TreeBuilderType;
@@ -197,6 +199,10 @@ moduleinit(PyObject* m)
if (PyType_Ready(&DiffType) < 0)
return NULL;
+ if (PyType_Ready(&DiffIterType) < 0)
+ return NULL;
+ if (PyType_Ready(&DiffEntryType) < 0)
+ return NULL;
if (PyType_Ready(&HunkType) < 0)
return NULL;
diff --git a/src/tree.c b/src/tree.c
index 0b22eee..7493299 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -346,8 +346,7 @@ Tree_diff(Tree *self, PyObject *args)
if (py_diff) {
Py_INCREF(self->repo);
py_diff->repo = self->repo;
- py_diff->diff = diff;
- py_diff->diff_changes = NULL;
+ py_diff->list = diff;
}
return (PyObject*)py_diff;
diff --git a/test/test_diff.py b/test/test_diff.py
index 03f49e5..9b3b3d2 100644
--- a/test/test_diff.py
+++ b/test/test_diff.py
@@ -31,6 +31,7 @@ from __future__ import absolute_import
from __future__ import unicode_literals
import unittest
import pygit2
+import itertools
from pygit2 import GIT_DIFF_INCLUDE_UNMODIFIED
from . import utils
@@ -86,16 +87,16 @@ class DiffDirtyTest(utils.DirtyRepoTestCase):
head = repo[repo.lookup_reference('HEAD').resolve().oid]
diff = head.tree.diff(repo.index)
- files = [x[1] for x in diff.changes['files']]
- self.assertEqual(DIFF_INDEX_EXPECTED, files)
+ files = [[x[0] for x in entry.files] for entry in diff]
+ self.assertEqual(DIFF_INDEX_EXPECTED, list(itertools.chain(*files)))
def test_workdir_to_tree(self):
repo = self.repo
head = repo[repo.lookup_reference('HEAD').resolve().oid]
diff = head.tree.diff()
- files = [x[1] for x in diff.changes['files']]
- self.assertEqual(DIFF_WORKDIR_EXPECTED, files)
+ files = [[x[0] for x in entry.files] for entry in diff]
+ self.assertEqual(DIFF_WORKDIR_EXPECTED, list(itertools.chain(*files)))
class DiffTest(utils.BareRepoTestCase):
@@ -109,8 +110,8 @@ class DiffTest(utils.BareRepoTestCase):
head = repo[repo.lookup_reference('HEAD').resolve().oid]
diff = head.tree.diff(repo.index)
- files = [x[0].split('/')[0] for x in diff.changes['files']]
- self.assertEqual([x.name for x in head.tree], files)
+ files = [[x[0].split('/')[0] for x in entry.files] for entry in diff]
+ self.assertEqual([x.name for x in head.tree], list(itertools.chain(*files)))
def test_diff_tree(self):
commit_a = self.repo[COMMIT_SHA1_1]
@@ -121,10 +122,10 @@ class DiffTest(utils.BareRepoTestCase):
# self.assertIsNotNone is 2.7 only
self.assertTrue(diff is not None)
# self.assertIn is 2.7 only
- self.assertTrue(('a', 'a', 3, 0) in diff.changes['files'])
- self.assertEqual(2, len(diff.changes['hunks']))
+ self.assertAny(lambda x: ('a', 'a', 3, 0) in x.files, diff)
+ self.assertEqual(2, sum(map(lambda x: len(x.hunks), diff)))
- hunk = diff.changes['hunks'][0]
+ hunk = diff[0].hunks[0]
self.assertEqual(hunk.old_start, 1)
self.assertEqual(hunk.old_lines, 1)
self.assertEqual(hunk.new_start, 1)
@@ -144,11 +145,11 @@ class DiffTest(utils.BareRepoTestCase):
pygit2.GIT_DIFF_IGNORE_WHITESPACE_EOL]:
diff = commit_c.tree.diff(commit_d.tree, opt)
self.assertTrue(diff is not None)
- self.assertEqual(0, len(diff.changes.get('hunks', list())))
+ self.assertEqual(0, len(diff[0].hunks))
diff = commit_c.tree.diff(commit_d.tree)
self.assertTrue(diff is not None)
- self.assertEqual(1, len(diff.changes.get('hunks', list())))
+ self.assertEqual(1, len(diff[0].hunks))
def test_diff_merge(self):
commit_a = self.repo[COMMIT_SHA1_1]
@@ -164,15 +165,15 @@ class DiffTest(utils.BareRepoTestCase):
self.assertTrue(diff_c is not None)
# assertIn / assertNotIn are 2.7 only
- self.assertTrue(('b', 'b', 3, 0) not in diff_b.changes['files'])
- self.assertTrue(('b', 'b', 3, 0) in diff_c.changes['files'])
+ self.assertAll(lambda x:('b', 'b', 3, 0) not in x.files, diff_b)
+ self.assertAny(lambda x:('b', 'b', 3, 0) in x.files, diff_c)
diff_b.merge(diff_c)
# assertIn is 2.7 only
- self.assertTrue(('b', 'b', 3, 0) in diff_b.changes['files'])
+ self.assertAny(lambda x:('b', 'b', 3, 0) in x.files, diff_b)
- hunk = diff_b.changes['hunks'][1]
+ hunk = diff_b[1].hunks[0]
self.assertEqual(hunk.old_start, 1)
self.assertEqual(hunk.old_lines, 1)
self.assertEqual(hunk.new_start, 1)
@@ -196,13 +197,13 @@ class DiffTest(utils.BareRepoTestCase):
commit_b = self.repo[COMMIT_SHA1_2]
diff = commit_a.tree.diff(commit_b.tree)
- self.assertEqual(diff.changes['hunks'][0].header, "@@ -1 +1 @@\n")
+ self.assertEqual(diff[0].hunks[0].header, "@@ -1 +1 @@\n")
def test_diff_oids(self):
commit_a = self.repo[COMMIT_SHA1_1]
commit_b = self.repo[COMMIT_SHA1_2]
diff = commit_a.tree.diff(commit_b.tree)
- hunk = diff.changes['hunks'][0]
+ hunk = diff[0].hunks[0]
self.assertEqual(hunk.old_oid,
'7f129fd57e31e935c6d60a0c794efe4e6927664b')
self.assertEqual(hunk.new_oid,
@@ -215,9 +216,9 @@ class DiffTest(utils.BareRepoTestCase):
#~ Must pass GIT_DIFF_INCLUDE_UNMODIFIED if you expect to emulate
#~ --find-copies-harder during rename transformion...
diff = commit_a.tree.diff(commit_b.tree, GIT_DIFF_INCLUDE_UNMODIFIED)
- self.assertFalse(('a', 'a.copy', 5, 100) in diff.changes['files'])
+ self.assertFalse(('a', 'a.copy', 5, 100) in diff[0].files)
diff.find_similar(pygit2.GIT_DIFF_FIND_COPIES_FROM_UNMODIFIED)
- self.assertTrue(('a', 'a.copy', 5, 100) in diff.changes['files'])
+ self.assertAny(lambda x:('a', 'a.copy', 5, 100) in x.files, diff)
if __name__ == '__main__':
unittest.main()