combine-diff: respect textconv attributes

When doing a combined diff, we did not respect textconv attributes at all. This generally lead to us printing "Binary files differ" when we could show a combined diff of the converted text. This patch converts file contents according to textconv attributes. The implementation is slightly ugly; because the textconv code is tightly linked with the diff_filespec code, we temporarily create a diff_filespec during conversion. In practice, though, this should not create a performance problem. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 23, 2011 at 16:31 UTC 0508fe533dfe1b890f6a2d31ca42ba25466e8ff5
2 files changed +131 -9
combine-diff.c
+32 -9
@@ -93,7 +93,9 @@ struct sline {
93 unsigned long *p_lno;
94 };
95
96 -static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned long *size)
96 +static char *grab_blob(const unsigned char *sha1, unsigned int mode,
97 + unsigned long *size, struct userdiff_driver *textconv,
98 + const char *path)
99 {
100 char *blob;
101 enum object_type type;
@@ -106,6 +108,11 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned lo
108 /* deleted blob */
109 *size = 0;
110 return xcalloc(1, 1);
111 + } else if (textconv) {
112 + struct diff_filespec *df = alloc_filespec(path);
113 + fill_filespec(df, sha1, mode);
114 + *size = fill_textconv(textconv, df, &blob);
115 + free_filespec(df);
116 } else {
117 blob = read_sha1_file(sha1, &type, size);
118 if (type != OBJ_BLOB)
@@ -205,7 +212,9 @@ static void consume_line(void *state_, char *line, unsigned long len)
212 static void combine_diff(const unsigned char *parent, unsigned int mode,
213 mmfile_t *result_file,
214 struct sline *sline, unsigned int cnt, int n,
208 - int num_parent, int result_deleted)
215 + int num_parent, int result_deleted,
216 + struct userdiff_driver *textconv,
217 + const char *path)
218 {
219 unsigned int p_lno, lno;
220 unsigned long nmask = (1UL << n);
@@ -218,7 +227,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
227 if (result_deleted)
228 return; /* result deleted */
229
221 - parent_file.ptr = grab_blob(parent, mode, &sz);
230 + parent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);
231 parent_file.size = sz;
232 memset(&xpp, 0, sizeof(xpp));
233 xpp.flags = 0;
@@ -771,16 +780,20 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
780 int working_tree_file = is_null_sha1(elem->sha1);
781 mmfile_t result_file;
782 struct userdiff_driver *userdiff;
783 + struct userdiff_driver *textconv = NULL;
784 int is_binary;
785
786 context = opt->context;
787 userdiff = userdiff_find_by_path(elem->path);
788 if (!userdiff)
789 userdiff = userdiff_find_by_name("default");
790 + if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV))
791 + textconv = userdiff_get_textconv(userdiff);
792
793 /* Read the result of merge first */
794 if (!working_tree_file)
783 - result = grab_blob(elem->sha1, elem->mode, &result_size);
795 + result = grab_blob(elem->sha1, elem->mode, &result_size,
796 + textconv, elem->path);
797 else {
798 /* Used by diff-tree to read from the working tree */
799 struct stat st;
@@ -803,9 +816,16 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
816 } else if (S_ISDIR(st.st_mode)) {
817 unsigned char sha1[20];
818 if (resolve_gitlink_ref(elem->path, "HEAD", sha1) < 0)
806 - result = grab_blob(elem->sha1, elem->mode, &result_size);
819 + result = grab_blob(elem->sha1, elem->mode,
820 + &result_size, NULL, NULL);
821 else
808 - result = grab_blob(sha1, elem->mode, &result_size);
822 + result = grab_blob(sha1, elem->mode,
823 + &result_size, NULL, NULL);
824 + } else if (textconv) {
825 + struct diff_filespec *df = alloc_filespec(elem->path);
826 + fill_filespec(df, null_sha1, st.st_mode);
827 + result_size = fill_textconv(textconv, df, &result);
828 + free_filespec(df);
829 } else if (0 <= (fd = open(elem->path, O_RDONLY))) {
830 size_t len = xsize_t(st.st_size);
831 ssize_t done;
@@ -862,7 +882,9 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
882 }
883 }
884
865 - if (userdiff->binary != -1)
885 + if (textconv)
886 + is_binary = 0;
887 + else if (userdiff->binary != -1)
888 is_binary = userdiff->binary;
889 else {
890 is_binary = buffer_is_binary(result, result_size);
@@ -871,7 +893,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
893 unsigned long size;
894 buf = grab_blob(elem->parent[i].sha1,
895 elem->parent[i].mode,
874 - &size);
896 + &size, NULL, NULL);
897 if (buffer_is_binary(buf, size))
898 is_binary = 1;
899 free(buf);
@@ -932,7 +954,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
954 combine_diff(elem->parent[i].sha1,
955 elem->parent[i].mode,
956 &result_file, sline,
935 - cnt, i, num_parent, result_deleted);
957 + cnt, i, num_parent, result_deleted,
958 + textconv, elem->path);
959 }
960
961 show_hunks = make_hunks(sline, cnt, num_parent, dense);
t/t4048-diff-combined-binary.sh
+99
@@ -110,4 +110,103 @@ test_expect_success 'diff --cc respects binary attribute' '
110 test_cmp expect actual
111 '
112
113 +test_expect_success 'setup textconv attribute' '
114 + echo "text diff=upcase" >.gitattributes &&
115 + git config diff.upcase.textconv "tr a-z A-Z <"
116 +'
117 +
118 +cat >expect <<'EOF'
119 +resolved
120 +
121 +diff --git a/text b/text
122 +index 2bdf67a..2ab19ae 100644
123 +--- a/text
124 ++++ b/text
125 +@@ -1 +1 @@
126 +-THREE
127 ++RESOLVED
128 +resolved
129 +
130 +diff --git a/text b/text
131 +index f719efd..2ab19ae 100644
132 +--- a/text
133 ++++ b/text
134 +@@ -1 +1 @@
135 +-TWO
136 ++RESOLVED
137 +EOF
138 +test_expect_success 'diff -m respects textconv attribute' '
139 + git show --format=%s -m >actual &&
140 + test_cmp expect actual
141 +'
142 +
143 +cat >expect <<'EOF'
144 +resolved
145 +
146 +diff --combined text
147 +index 2bdf67a,f719efd..2ab19ae
148 +--- a/text
149 ++++ b/text
150 +@@@ -1,1 -1,1 +1,1 @@@
151 +- THREE
152 + -TWO
153 +++RESOLVED
154 +EOF
155 +test_expect_success 'diff -c respects textconv attribute' '
156 + git show --format=%s -c >actual &&
157 + test_cmp expect actual
158 +'
159 +
160 +cat >expect <<'EOF'
161 +resolved
162 +
163 +diff --cc text
164 +index 2bdf67a,f719efd..2ab19ae
165 +--- a/text
166 ++++ b/text
167 +@@@ -1,1 -1,1 +1,1 @@@
168 +- THREE
169 + -TWO
170 +++RESOLVED
171 +EOF
172 +test_expect_success 'diff --cc respects textconv attribute' '
173 + git show --format=%s --cc >actual &&
174 + test_cmp expect actual
175 +'
176 +
177 +cat >expect <<'EOF'
178 +diff --combined text
179 +index 2bdf67a,f719efd..2ab19ae
180 +--- a/text
181 ++++ b/text
182 +@@@ -1,1 -1,1 +1,1 @@@
183 +- three
184 + -two
185 +++resolved
186 +EOF
187 +test_expect_success 'diff-tree plumbing does not respect textconv' '
188 + git diff-tree HEAD -c -p >full &&
189 + tail -n +2 full >actual &&
190 + test_cmp expect actual
191 +'
192 +
193 +cat >expect <<'EOF'
194 +diff --cc text
195 +index 2bdf67a,f719efd..0000000
196 +--- a/text
197 ++++ b/text
198 +@@@ -1,1 -1,1 +1,5 @@@
199 +++<<<<<<< HEAD
200 + +THREE
201 +++=======
202 ++ TWO
203 +++>>>>>>> MASTER
204 +EOF
205 +test_expect_success 'diff --cc respects textconv on worktree file' '
206 + git reset --hard HEAD^ &&
207 + test_must_fail git merge master &&
208 + git diff >actual &&
209 + test_cmp expect actual
210 +'
211 +
212 test_done