Commit 3940310e29363978ccdc1f3b557bc6f48ebae8f0
2013-10-30T13:56:42
Fix some of the glaring errors in GIT_DIFF_REVERSE These changes fix the basic problem with GIT_DIFF_REVERSE being broken for text diffs. The reversed diff entries were getting added to the git_diff correctly, but some of the metadata was kept incorrectly in a way that prevented the text diffs from being generated correctly. Once I fixed that, it became clear that it was not possible to merge reversed diffs correctly. This has a first pass at fixing that problem. We probably need more tests to make sure that is really fixed thoroughly.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110
diff --git a/src/diff.c b/src/diff.c
index 37bc737..b1f64e6 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -448,6 +448,13 @@ static int diff_list_apply_options(
/* add other defaults here */
}
+ /* Reverse src info if diff is reversed */
+ if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
+ git_iterator_type_t tmp_src = diff->old_src;
+ diff->old_src = diff->new_src;
+ diff->new_src = tmp_src;
+ }
+
/* if ignore_submodules not explicitly set, check diff config */
if (diff->opts.ignore_submodules <= 0) {
const char *str;
@@ -484,9 +491,9 @@ static int diff_list_apply_options(
return -1;
if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
- const char *swap = diff->opts.old_prefix;
- diff->opts.old_prefix = diff->opts.new_prefix;
- diff->opts.new_prefix = swap;
+ const char *tmp_prefix = diff->opts.old_prefix;
+ diff->opts.old_prefix = diff->opts.new_prefix;
+ diff->opts.new_prefix = tmp_prefix;
}
return 0;
diff --git a/src/diff_tform.c b/src/diff_tform.c
index 9461ca2..0aec754 100644
--- a/src/diff_tform.c
+++ b/src/diff_tform.c
@@ -46,7 +46,10 @@ fail:
}
static git_diff_delta *diff_delta__merge_like_cgit(
- const git_diff_delta *a, const git_diff_delta *b, git_pool *pool)
+ uint16_t flags,
+ const git_diff_delta *a,
+ const git_diff_delta *b,
+ git_pool *pool)
{
git_diff_delta *dup;
@@ -112,26 +115,28 @@ int git_diff_merge(
if (!from->deltas.length)
return 0;
+ if ((onto->opts.flags & GIT_DIFF_IGNORE_CASE) !=
+ (from->opts.flags & GIT_DIFF_IGNORE_CASE) ||
+ (onto->opts.flags & GIT_DIFF_REVERSE) !=
+ (from->opts.flags & GIT_DIFF_REVERSE))
+ {
+ giterr_set(GITERR_INVALID,
+ "Attempt to merge diffs created with conflicting options");
+ return -1;
+ }
+
if (git_vector_init(
&onto_new, onto->deltas.length, git_diff_delta__cmp) < 0 ||
git_pool_init(&onto_pool, 1, 0) < 0)
return -1;
- if ((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0 ||
- (from->opts.flags & GIT_DIFF_IGNORE_CASE) != 0)
- {
- ignore_case = true;
-
- /* This function currently only supports merging diff lists that
- * are sorted identically. */
- assert((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0 &&
- (from->opts.flags & GIT_DIFF_IGNORE_CASE) != 0);
- }
+ ignore_case = ((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0);
for (i = 0, j = 0; i < onto->deltas.length || j < from->deltas.length; ) {
git_diff_delta *o = GIT_VECTOR_GET(&onto->deltas, i);
const git_diff_delta *f = GIT_VECTOR_GET(&from->deltas, j);
- int cmp = !f ? -1 : !o ? 1 : STRCMP_CASESELECT(ignore_case, o->old_file.path, f->old_file.path);
+ int cmp = !f ? -1 : !o ? 1 :
+ STRCMP_CASESELECT(ignore_case, o->old_file.path, f->old_file.path);
if (cmp < 0) {
delta = diff_delta__dup(o, &onto_pool);
@@ -140,7 +145,8 @@ int git_diff_merge(
delta = diff_delta__dup(f, &onto_pool);
j++;
} else {
- delta = diff_delta__merge_like_cgit(o, f, &onto_pool);
+ delta = diff_delta__merge_like_cgit(
+ onto->opts.flags, o, f, &onto_pool);
i++;
j++;
}
@@ -160,7 +166,11 @@ int git_diff_merge(
if (!error) {
git_vector_swap(&onto->deltas, &onto_new);
git_pool_swap(&onto->pool, &onto_pool);
- onto->new_src = from->new_src;
+
+ if ((onto->opts.flags & GIT_DIFF_REVERSE) != 0)
+ onto->old_src = from->old_src;
+ else
+ onto->new_src = from->new_src;
/* prefix strings also come from old pool, so recreate those.*/
onto->opts.old_prefix =