Fix diff patch line number calculation This was just wrong. Added a test that verifying patch line numbers even for hunks further into a file and then fixed the algorithm. I needed to add a little extra state into the patch so that I could track old and new file numbers independently, but it should be okay.
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 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211
diff --git a/src/diff_output.c b/src/diff_output.c
index f98665d..d75a7bb 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -821,6 +821,9 @@ static int diff_patch_hunk_cb(
hunk->line_start = patch->lines_size;
hunk->line_count = 0;
+ patch->oldno = range->old_start;
+ patch->newno = range->new_start;
+
return 0;
}
@@ -879,24 +882,23 @@ static int diff_patch_line_cb(
++line->lines;
}
- if (!last) {
- line->oldno = hunk->range.old_start;
- line->newno = hunk->range.new_start;
- } else {
- switch (last->origin) {
- case GIT_DIFF_LINE_ADDITION:
- line->oldno = last->oldno;
- line->newno = last->newno + last->lines;
- break;
- case GIT_DIFF_LINE_DELETION:
- line->oldno = last->oldno + last->lines;
- line->newno = last->newno;
- break;
- default:
- line->oldno = last->oldno + last->lines;
- line->newno = last->newno + last->lines;
- break;
- }
+ switch (line_origin) {
+ case GIT_DIFF_LINE_ADDITION:
+ line->oldno = -1;
+ line->newno = patch->newno;
+ patch->newno += line->lines;
+ break;
+ case GIT_DIFF_LINE_DELETION:
+ line->oldno = patch->oldno;
+ line->newno = -1;
+ patch->oldno += line->lines;
+ break;
+ default:
+ line->oldno = patch->oldno;
+ line->newno = patch->newno;
+ patch->oldno += line->lines;
+ patch->newno += line->lines;
+ break;
}
hunk->line_count++;
diff --git a/src/diff_output.h b/src/diff_output.h
index 7785bf5..ae1a491 100644
--- a/src/diff_output.h
+++ b/src/diff_output.h
@@ -72,6 +72,7 @@ struct git_diff_patch {
size_t hunks_asize, hunks_size;
diff_patch_line *lines;
size_t lines_asize, lines_size;
+ size_t oldno, newno;
};
/* context for performing diff on a single delta */
diff --git a/tests-clar/diff/patch.c b/tests-clar/diff/patch.c
index 6a3c5bc..4e85ab8 100644
--- a/tests-clar/diff/patch.c
+++ b/tests-clar/diff/patch.c
@@ -1,11 +1,12 @@
#include "clar_libgit2.h"
#include "diff_helpers.h"
+#include "repository.h"
+#include "buf_text.h"
static git_repository *g_repo = NULL;
void test_diff_patch__initialize(void)
{
- g_repo = cl_git_sandbox_init("status");
}
void test_diff_patch__cleanup(void)
@@ -85,6 +86,8 @@ void test_diff_patch__can_properly_display_the_removal_of_a_file(void)
git_tree *one, *another;
git_diff_list *diff;
+ g_repo = cl_git_sandbox_init("status");
+
one = resolve_commit_oid_to_tree(g_repo, one_sha);
another = resolve_commit_oid_to_tree(g_repo, another_sha);
@@ -108,6 +111,8 @@ void test_diff_patch__to_string(void)
char *text;
const char *expected = "diff --git a/subdir.txt b/subdir.txt\ndeleted file mode 100644\nindex e8ee89e..0000000\n--- a/subdir.txt\n+++ /dev/null\n@@ -1,2 +0,0 @@\n-Is it a bird?\n-Is it a plane?\n";
+ g_repo = cl_git_sandbox_init("status");
+
one = resolve_commit_oid_to_tree(g_repo, one_sha);
another = resolve_commit_oid_to_tree(g_repo, another_sha);
@@ -127,3 +132,106 @@ void test_diff_patch__to_string(void)
git_tree_free(another);
git_tree_free(one);
}
+
+void test_diff_patch__hunks_have_correct_line_numbers(void)
+{
+ git_tree *head;
+ git_diff_list *diff;
+ git_diff_patch *patch;
+ const git_diff_delta *delta;
+ const git_diff_range *range;
+ const char *hdr, *text;
+ size_t hdrlen, hunklen, textlen;
+ char origin;
+ int oldno, newno;
+ const char *new_content = "The Song of Seven Cities\n========================\n\nI WAS Lord of Cities very sumptuously builded.\nSeven roaring Cities paid me tribute from afar.\nIvory their outposts were—the guardrooms of them gilded,\nAnd garrisoned with Amazons invincible in war.\n\nThis is some new text;\nNot as good as the old text;\nBut here it is.\n\nSo they warred and trafficked only yesterday, my Cities.\nTo-day there is no mark or mound of where my Cities stood.\nFor the River rose at midnight and it washed away my Cities.\nThey are evened with Atlantis and the towns before the Flood.\n\nRain on rain-gorged channels raised the water-levels round them,\nFreshet backed on freshet swelled and swept their world from sight,\nTill the emboldened floods linked arms and, flashing forward, drowned them—\nDrowned my Seven Cities and their peoples in one night!\n\nLow among the alders lie their derelict foundations,\nThe beams wherein they trusted and the plinths whereon they built—\nMy rulers and their treasure and their unborn populations,\nDead, destroyed, aborted, and defiled with mud and silt!\n\nAnother replacement;\nBreaking up the poem;\nGenerating some hunks.\n\nTo the sound of trumpets shall their seed restore my Cities\nWealthy and well-weaponed, that once more may I behold\nAll the world go softly when it walks before my Cities,\nAnd the horses and the chariots fleeing from them as of old!\n\n -- Rudyard Kipling\n";
+
+ g_repo = cl_git_sandbox_init("renames");
+
+ cl_git_rewritefile("renames/songofseven.txt", new_content);
+
+ cl_git_pass(git_repository_head_tree(&head, g_repo));
+
+ cl_git_pass(git_diff_tree_to_workdir(&diff, g_repo, head, NULL));
+
+ cl_assert_equal_i(1, (int)git_diff_num_deltas(diff));
+
+ cl_git_pass(git_diff_get_patch(&patch, &delta, diff, 0));
+
+ cl_assert_equal_i(GIT_DELTA_MODIFIED, (int)delta->status);
+ cl_assert_equal_i(2, (int)git_diff_patch_num_hunks(patch));
+
+ /* check hunk 0 */
+
+ cl_git_pass(
+ git_diff_patch_get_hunk(&range, &hdr, &hdrlen, &hunklen, patch, 0));
+
+ cl_assert_equal_i(18, (int)hunklen);
+
+ cl_assert_equal_i(6, (int)range->old_start);
+ cl_assert_equal_i(15, (int)range->old_lines);
+ cl_assert_equal_i(6, (int)range->new_start);
+ cl_assert_equal_i(9, (int)range->new_lines);
+
+ cl_assert_equal_i(18, (int)git_diff_patch_num_lines_in_hunk(patch, 0));
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 0, 0));
+ cl_assert_equal_i(GIT_DIFF_LINE_CONTEXT, (int)origin);
+ cl_assert(strncmp("Ivory their outposts were—the guardrooms of them gilded,\n", text, textlen) == 0);
+ cl_assert_equal_i(6, oldno);
+ cl_assert_equal_i(6, newno);
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 0, 3));
+ cl_assert_equal_i(GIT_DIFF_LINE_DELETION, (int)origin);
+ cl_assert(strncmp("All the world went softly when it walked before my Cities—\n", text, textlen) == 0);
+ cl_assert_equal_i(9, oldno);
+ cl_assert_equal_i(-1, newno);
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 0, 12));
+ cl_assert_equal_i(GIT_DIFF_LINE_ADDITION, (int)origin);
+ cl_assert(strncmp("This is some new text;\n", text, textlen) == 0);
+ cl_assert_equal_i(-1, oldno);
+ cl_assert_equal_i(9, newno);
+
+ /* check hunk 1 */
+
+ cl_git_pass(
+ git_diff_patch_get_hunk(&range, &hdr, &hdrlen, &hunklen, patch, 1));
+
+ cl_assert_equal_i(18, (int)hunklen);
+
+ cl_assert_equal_i(31, (int)range->old_start);
+ cl_assert_equal_i(15, (int)range->old_lines);
+ cl_assert_equal_i(25, (int)range->new_start);
+ cl_assert_equal_i(9, (int)range->new_lines);
+
+ cl_assert_equal_i(18, (int)git_diff_patch_num_lines_in_hunk(patch, 1));
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 1, 0));
+ cl_assert_equal_i(GIT_DIFF_LINE_CONTEXT, (int)origin);
+ cl_assert(strncmp("My rulers and their treasure and their unborn populations,\n", text, textlen) == 0);
+ cl_assert_equal_i(31, oldno);
+ cl_assert_equal_i(25, newno);
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 1, 3));
+ cl_assert_equal_i(GIT_DIFF_LINE_DELETION, (int)origin);
+ cl_assert(strncmp("The Daughters of the Palace whom they cherished in my Cities,\n", text, textlen) == 0);
+ cl_assert_equal_i(34, oldno);
+ cl_assert_equal_i(-1, newno);
+
+ cl_git_pass(git_diff_patch_get_line_in_hunk(
+ &origin, &text, &textlen, &oldno, &newno, patch, 1, 12));
+ cl_assert_equal_i(GIT_DIFF_LINE_ADDITION, (int)origin);
+ cl_assert(strncmp("Another replacement;\n", text, textlen) == 0);
+ cl_assert_equal_i(-1, oldno);
+ cl_assert_equal_i(28, newno);
+
+ git_diff_patch_free(patch);
+ git_diff_list_free(diff);
+ git_tree_free(head);
+}