tree-diff: consolidate code for emitting diffs and recursion in one place
Currently both compare_tree_entry() and show_entry() invoke opt diff callbacks (opt->add_remove() and opt->change()), and also they both have code which decides whether to recurse into sub-tree, and whether to emit a tree as separate entry if DIFF_OPT_TREE_IN_RECURSIVE is set. I.e. we have code duplication and logic scattered on two places. Let's consolidate it - all diff emiting code and recurion logic moves to show_entry, which is now named as show_path, because it shows diff for a path, based on up to two tree entries, with actual diff emitting code being kept in new helper emit_diff() for clarity. What we have as the result, is that compare_tree_entry is now free from code with logic for diff generation, and also performance is not affected as timings for `git log --raw --no-abbrev --no-renames` for navy.git and `linux.git v3.10..v3.11`, just like in previous patch, stay the same. Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Kirill Smelkov committed
Feb 24, 2014 at 20:21 UTC
d00e980c224d4b65972dda4474fbd9294bdb185f
1 file changed
+82
-30
tree-diff.c
+82
-30
@@ -6,8 +6,8 @@
6
#include "diffcore.h"
7
#include "tree.h"
8
9
-static void show_entry(struct diff_options *opt, const char *prefix,
10
- struct tree_desc *desc, struct strbuf *base);
9
+static void show_path(struct strbuf *base, struct diff_options *opt,
10
+ struct tree_desc *t1, struct tree_desc *t2);
11
12
static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
13
struct strbuf *base, struct diff_options *opt)
@@ -16,7 +16,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
16
const char *path1, *path2;
17
const unsigned char *sha1, *sha2;
18
int cmp, pathlen1, pathlen2;
19
- int old_baselen = base->len;
19
20
sha1 = tree_entry_extract(t1, &path1, &mode1);
21
sha2 = tree_entry_extract(t2, &path2, &mode2);
@@ -30,51 +29,104 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2,
29
*/
30
cmp = base_name_compare(path1, pathlen1, mode1, path2, pathlen2, mode2);
31
if (cmp < 0) {
33
- show_entry(opt, "-", t1, base);
32
+ show_path(base, opt, t1, /*t2=*/NULL);
33
return -1;
34
}
35
if (cmp > 0) {
37
- show_entry(opt, "+", t2, base);
36
+ show_path(base, opt, /*t1=*/NULL, t2);
37
return 1;
38
}
39
if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
40
return 0;
41
43
- strbuf_add(base, path1, pathlen1);
44
- if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
45
- if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
46
- opt->change(opt, mode1, mode2,
47
- sha1, sha2, 1, 1, base->buf, 0, 0);
42
+ show_path(base, opt, t1, t2);
43
+ return 0;
44
+}
45
+
46
+
47
+/* convert path, t1/t2 -> opt->diff_*() callbacks */
48
+static void emit_diff(struct diff_options *opt, struct strbuf *path,
49
+ struct tree_desc *t1, struct tree_desc *t2)
50
+{
51
+ unsigned int mode1 = t1 ? t1->entry.mode : 0;
52
+ unsigned int mode2 = t2 ? t2->entry.mode : 0;
53
+
54
+ if (mode1 && mode2) {
55
+ opt->change(opt, mode1, mode2, t1->entry.sha1, t2->entry.sha1,
56
+ 1, 1, path->buf, 0, 0);
57
+ }
58
+ else {
59
+ const unsigned char *sha1;
60
+ unsigned int mode;
61
+ int addremove;
62
+
63
+ if (mode2) {
64
+ addremove = '+';
65
+ sha1 = t2->entry.sha1;
66
+ mode = mode2;
67
+ } else {
68
+ addremove = '-';
69
+ sha1 = t1->entry.sha1;
70
+ mode = mode1;
71
}
49
- strbuf_addch(base, '/');
50
- diff_tree_sha1(sha1, sha2, base->buf, opt);
51
- } else {
52
- opt->change(opt, mode1, mode2, sha1, sha2, 1, 1, base->buf, 0, 0);
72
+
73
+ opt->add_remove(opt, addremove, mode, sha1, 1, path->buf, 0);
74
}
54
- strbuf_setlen(base, old_baselen);
55
- return 0;
75
}
76
58
-/* An entry went away or appeared */
59
-static void show_entry(struct diff_options *opt, const char *prefix,
60
- struct tree_desc *desc, struct strbuf *base)
77
+
78
+/* new path should be added to diff
79
+ *
80
+ * 3 cases on how/when it should be called and behaves:
81
+ *
82
+ * !t1, t2 -> path added, parent lacks it
83
+ * t1, !t2 -> path removed from parent
84
+ * t1, t2 -> path modified
85
+ */
86
+static void show_path(struct strbuf *base, struct diff_options *opt,
87
+ struct tree_desc *t1, struct tree_desc *t2)
88
{
89
unsigned mode;
90
const char *path;
64
- const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
65
- int pathlen = tree_entry_len(&desc->entry);
91
+ int pathlen;
92
int old_baselen = base->len;
93
+ int isdir, recurse = 0, emitthis = 1;
94
+
95
+ /* at least something has to be valid */
96
+ assert(t1 || t2);
97
+
98
+ if (t2) {
99
+ /* path present in resulting tree */
100
+ tree_entry_extract(t2, &path, &mode);
101
+ pathlen = tree_entry_len(&t2->entry);
102
+ isdir = S_ISDIR(mode);
103
+ } else {
104
+ /*
105
+ * a path was removed - take path from parent. Also take
106
+ * mode from parent, to decide on recursion.
107
+ */
108
+ tree_entry_extract(t1, &path, &mode);
109
+ pathlen = tree_entry_len(&t1->entry);
110
+
111
+ isdir = S_ISDIR(mode);
112
+ mode = 0;
113
+ }
114
+
115
+ if (DIFF_OPT_TST(opt, RECURSIVE) && isdir) {
116
+ recurse = 1;
117
+ emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE);
118
+ }
119
120
strbuf_add(base, path, pathlen);
69
- if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
70
- if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
71
- opt->add_remove(opt, *prefix, mode, sha1, 1, base->buf, 0);
121
122
+ if (emitthis)
123
+ emit_diff(opt, base, t1, t2);
124
+
125
+ if (recurse) {
126
strbuf_addch(base, '/');
74
- diff_tree_sha1(*prefix == '-' ? sha1 : NULL,
75
- *prefix == '+' ? sha1 : NULL, base->buf, opt);
76
- } else
77
- opt->add_remove(opt, prefix[0], mode, sha1, 1, base->buf, 0);
127
+ diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
128
+ t2 ? t2->entry.sha1 : NULL, base->buf, opt);
129
+ }
130
131
strbuf_setlen(base, old_baselen);
132
}
@@ -117,12 +169,12 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
169
if (!t1->size) {
170
if (!t2->size)
171
break;
120
- show_entry(opt, "+", t2, &base);
172
+ show_path(&base, opt, /*t1=*/NULL, t2);
173
update_tree_entry(t2);
174
continue;
175
}
176
if (!t2->size) {
125
- show_entry(opt, "-", t1, &base);
177
+ show_path(&base, opt, t1, /*t2=*/NULL);
178
update_tree_entry(t1);
179
continue;
180
}