From: Jeff King <peff@peff.net>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 5/5] combine-diff: respect textconv attributes
Date: Mon, 23 May 2011 16:31:05 -0400 [thread overview]
Message-ID: <20110523203105.GE6298@sigill.intra.peff.net> (raw)
In-Reply-To: <20110523201529.GA6281@sigill.intra.peff.net>
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>
---
combine-diff.c | 41 +++++++++++++----
t/t4048-diff-combined-binary.sh | 99 +++++++++++++++++++++++++++++++++++++++
2 files changed, 131 insertions(+), 9 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 94a207f..fbed374 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -93,7 +93,8 @@ struct sline {
unsigned long *p_lno;
};
-static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned long *size)
+static char *grab_blob(const unsigned char *sha1, unsigned int mode,
+ unsigned long *size, struct userdiff_driver *textconv)
{
char *blob;
enum object_type type;
@@ -106,6 +107,13 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode, unsigned lo
/* deleted blob */
*size = 0;
return xcalloc(1, 1);
+ } else if (textconv) {
+ /* yuck, the textconv code is linked heavily with
+ * filespecs */
+ struct diff_filespec *df = alloc_filespec("");
+ fill_filespec(df, sha1, mode);
+ *size = fill_textconv(textconv, df, &blob);
+ free_filespec(df);
} else {
blob = read_sha1_file(sha1, &type, size);
if (type != OBJ_BLOB)
@@ -205,7 +213,8 @@ static void consume_line(void *state_, char *line, unsigned long len)
static void combine_diff(const unsigned char *parent, unsigned int mode,
mmfile_t *result_file,
struct sline *sline, unsigned int cnt, int n,
- int num_parent, int result_deleted)
+ int num_parent, int result_deleted,
+ struct userdiff_driver *textconv)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL << n);
@@ -218,7 +227,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
if (result_deleted)
return; /* result deleted */
- parent_file.ptr = grab_blob(parent, mode, &sz);
+ parent_file.ptr = grab_blob(parent, mode, &sz, textconv);
parent_file.size = sz;
memset(&xpp, 0, sizeof(xpp));
xpp.flags = 0;
@@ -771,16 +780,20 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
int working_tree_file = is_null_sha1(elem->sha1);
mmfile_t result_file;
struct userdiff_driver *userdiff;
+ struct userdiff_driver *textconv = NULL;
int is_binary;
context = opt->context;
userdiff = userdiff_find_by_path(elem->path);
if (!userdiff)
userdiff = userdiff_find_by_name("default");
+ if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV))
+ textconv = userdiff_get_textconv(userdiff);
/* Read the result of merge first */
if (!working_tree_file)
- result = grab_blob(elem->sha1, elem->mode, &result_size);
+ result = grab_blob(elem->sha1, elem->mode, &result_size,
+ textconv);
else {
/* Used by diff-tree to read from the working tree */
struct stat st;
@@ -803,9 +816,16 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
} else if (S_ISDIR(st.st_mode)) {
unsigned char sha1[20];
if (resolve_gitlink_ref(elem->path, "HEAD", sha1) < 0)
- result = grab_blob(elem->sha1, elem->mode, &result_size);
+ result = grab_blob(elem->sha1, elem->mode,
+ &result_size, NULL);
else
- result = grab_blob(sha1, elem->mode, &result_size);
+ result = grab_blob(sha1, elem->mode,
+ &result_size, NULL);
+ } else if (textconv) {
+ struct diff_filespec *df = alloc_filespec(elem->path);
+ fill_filespec(df, null_sha1, st.st_mode);
+ result_size = fill_textconv(textconv, df, &result);
+ free_filespec(df);
} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
size_t len = xsize_t(st.st_size);
ssize_t done;
@@ -862,7 +882,9 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
}
}
- if (userdiff->binary != -1)
+ if (textconv)
+ is_binary = 0;
+ else if (userdiff->binary != -1)
is_binary = userdiff->binary;
else {
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,
unsigned long size;
buf = grab_blob(elem->parent[i].sha1,
elem->parent[i].mode,
- &size);
+ &size, NULL);
if (buffer_is_binary(buf, size))
is_binary = 1;
free(buf);
@@ -932,7 +954,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
combine_diff(elem->parent[i].sha1,
elem->parent[i].mode,
&result_file, sline,
- cnt, i, num_parent, result_deleted);
+ cnt, i, num_parent, result_deleted,
+ textconv);
}
show_hunks = make_hunks(sline, cnt, num_parent, dense);
diff --git a/t/t4048-diff-combined-binary.sh b/t/t4048-diff-combined-binary.sh
index a943994..87a8949 100755
--- a/t/t4048-diff-combined-binary.sh
+++ b/t/t4048-diff-combined-binary.sh
@@ -110,4 +110,103 @@ test_expect_success 'diff --cc respects binary attribute' '
test_cmp expect actual
'
+test_expect_success 'setup textconv attribute' '
+ echo "text diff=upcase" >.gitattributes &&
+ git config diff.upcase.textconv "tr a-z A-Z <"
+'
+
+cat >expect <<'EOF'
+resolved
+
+diff --git a/text b/text
+index 2bdf67a..2ab19ae 100644
+--- a/text
++++ b/text
+@@ -1 +1 @@
+-THREE
++RESOLVED
+resolved
+
+diff --git a/text b/text
+index f719efd..2ab19ae 100644
+--- a/text
++++ b/text
+@@ -1 +1 @@
+-TWO
++RESOLVED
+EOF
+test_expect_success 'diff -m respects textconv attribute' '
+ git show --format=%s -m >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+resolved
+
+diff --combined text
+index 2bdf67a,f719efd..2ab19ae
+--- a/text
++++ b/text
+@@@ -1,1 -1,1 +1,1 @@@
+- THREE
+ -TWO
+++RESOLVED
+EOF
+test_expect_success 'diff -c respects textconv attribute' '
+ git show --format=%s -c >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+resolved
+
+diff --cc text
+index 2bdf67a,f719efd..2ab19ae
+--- a/text
++++ b/text
+@@@ -1,1 -1,1 +1,1 @@@
+- THREE
+ -TWO
+++RESOLVED
+EOF
+test_expect_success 'diff --cc respects textconv attribute' '
+ git show --format=%s --cc >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+diff --combined text
+index 2bdf67a,f719efd..2ab19ae
+--- a/text
++++ b/text
+@@@ -1,1 -1,1 +1,1 @@@
+- three
+ -two
+++resolved
+EOF
+test_expect_success 'diff-tree plumbing does not respect textconv' '
+ git diff-tree HEAD -c -p >full &&
+ tail -n +2 full >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+diff --cc text
+index 2bdf67a,f719efd..0000000
+--- a/text
++++ b/text
+@@@ -1,1 -1,1 +1,5 @@@
+++<<<<<<< HEAD
+ +THREE
+++=======
++ TWO
+++>>>>>>> MASTER
+EOF
+test_expect_success 'diff --cc respects textconv on worktree file' '
+ git reset --hard HEAD^ &&
+ test_must_fail git merge master &&
+ git diff >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.5.2.4.g43415
next prev parent reply other threads:[~2011-05-23 20:31 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-22 20:12 combined diff does not detect binary files and ignores -diff attribute Jay Soffian
2011-05-23 13:30 ` Michael J Gruber
2011-05-23 15:17 ` Jay Soffian
2011-05-23 17:07 ` Junio C Hamano
2011-05-23 18:11 ` Jeff King
2011-05-23 20:15 ` Jeff King
2011-05-23 20:16 ` [PATCH 1/5] combine-diff: split header printing into its own function Jeff King
2011-05-23 20:16 ` [PATCH 2/5] combine-diff: calculate mode_differs earlier Jeff King
2011-05-23 20:27 ` [PATCH 3/5] combine-diff: handle binary files as binary Jeff King
2011-05-23 23:02 ` Junio C Hamano
2011-05-23 23:50 ` Jeff King
2011-05-30 6:33 ` Junio C Hamano
2011-05-30 14:36 ` Jeff King
2011-05-30 16:19 ` Jeff King
2011-05-30 19:32 ` Junio C Hamano
2011-05-31 22:42 ` Junio C Hamano
2011-05-23 20:30 ` [PATCH 4/5] refactor get_textconv to not require diff_filespec Jeff King
2011-05-23 20:31 ` Jeff King [this message]
2011-05-23 22:47 ` [PATCH 5/5] combine-diff: respect textconv attributes Junio C Hamano
2011-05-23 23:39 ` Jeff King
2011-05-24 16:20 ` Junio C Hamano
2011-05-24 18:52 ` Jeff King
2011-05-23 22:55 ` combined diff does not detect binary files and ignores -diff attribute Jay Soffian
2011-05-23 23:31 ` Jay Soffian
2011-05-23 23:49 ` Jeff King
2011-05-24 0:59 ` Jay Soffian
2011-05-23 23:41 ` Jeff King
2011-05-24 4:46 ` Junio C Hamano
2011-05-24 7:19 ` Michael J Gruber
2011-05-24 15:36 ` Junio C Hamano
2011-05-24 16:38 ` Michael J Gruber
2011-05-24 16:43 ` Junio C Hamano
2011-05-24 16:52 ` Jay Soffian
2011-05-24 19:13 ` Jeff King
2011-05-25 7:38 ` Michael J Gruber
2011-05-25 15:29 ` Jeff King
2011-05-24 14:40 ` Jay Soffian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110523203105.GE6298@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jaysoffian@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).