Skip to content

Commit 0508fe5

Browse files
peffgitster
peff
authored and
gitster
committed
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>
1 parent 3813e69 commit 0508fe5

File tree

2 files changed

+131
-9
lines changed

2 files changed

+131
-9
lines changed

‎combine-diff.c‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ struct sline {
9393
unsigned long *p_lno;
9494
};
9595

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)
9799
{
98100
char *blob;
99101
enum object_type type;
@@ -106,6 +108,11 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned lo
106108
/* deleted blob */
107109
*size = 0;
108110
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);
109116
} else {
110117
blob = read_sha1_file(sha1, &type, size);
111118
if (type != OBJ_BLOB)
@@ -205,7 +212,9 @@ static void consume_line(void *state_, char *line, unsigned long len)
205212
static void combine_diff(const unsigned char *parent, unsigned int mode,
206213
mmfile_t *result_file,
207214
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)
209218
{
210219
unsigned int p_lno, lno;
211220
unsigned long nmask = (1UL << n);
@@ -218,7 +227,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
218227
if (result_deleted)
219228
return; /* result deleted */
220229

221-
parent_file.ptr = grab_blob(parent, mode, &sz);
230+
parent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);
222231
parent_file.size = sz;
223232
memset(&xpp, 0, sizeof(xpp));
224233
xpp.flags = 0;
@@ -771,16 +780,20 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
771780
int working_tree_file = is_null_sha1(elem->sha1);
772781
mmfile_t result_file;
773782
struct userdiff_driver *userdiff;
783+
struct userdiff_driver *textconv = NULL;
774784
int is_binary;
775785

776786
context = opt->context;
777787
userdiff = userdiff_find_by_path(elem->path);
778788
if (!userdiff)
779789
userdiff = userdiff_find_by_name("default");
790+
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV))
791+
textconv = userdiff_get_textconv(userdiff);
780792

781793
/* Read the result of merge first */
782794
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);
784797
else {
785798
/* Used by diff-tree to read from the working tree */
786799
struct stat st;
@@ -803,9 +816,16 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
803816
} else if (S_ISDIR(st.st_mode)) {
804817
unsigned char sha1[20];
805818
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);
807821
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);
809829
} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
810830
size_t len = xsize_t(st.st_size);
811831
ssize_t done;
@@ -862,7 +882,9 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
862882
}
863883
}
864884

865-
if (userdiff->binary != -1)
885+
if (textconv)
886+
is_binary = 0;
887+
else if (userdiff->binary != -1)
866888
is_binary = userdiff->binary;
867889
else {
868890
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,
871893
unsigned long size;
872894
buf = grab_blob(elem->parent[i].sha1,
873895
elem->parent[i].mode,
874-
&size);
896+
&size, NULL, NULL);
875897
if (buffer_is_binary(buf, size))
876898
is_binary = 1;
877899
free(buf);
@@ -932,7 +954,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
932954
combine_diff(elem->parent[i].sha1,
933955
elem->parent[i].mode,
934956
&result_file, sline,
935-
cnt, i, num_parent, result_deleted);
957+
cnt, i, num_parent, result_deleted,
958+
textconv, elem->path);
936959
}
937960

938961
show_hunks = make_hunks(sline, cnt, num_parent, dense);

‎t/t4048-diff-combined-binary.sh‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,103 @@ test_expect_success 'diff --cc respects binary attribute' '
110110
test_cmp expect actual
111111
'
112112

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+
113212
test_done

0 commit comments

Comments
 (0)