From: Johannes Sixt <j.sixt@viscovery.net>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [PATCH 3/5] refactor userdiff textconv code
Date: Fri, 24 Oct 2008 09:15:54 +0200 [thread overview]
Message-ID: <4901762A.3090003@viscovery.net> (raw)
In-Reply-To: <20081024025330.GC2831@coredump.intra.peff.net>
Jeff King schrieb:
> This patch moves the text conversion to a
> diff_filespec_textconv function; this function can be called
> to insert the text-converted contents of the file into the
> filespec. These contents will survive through any
> diff_populate_filespec calls, meaning that it is safe to
> pass the resulting filespec to other diff functions which
> will look at the content.
What do we do when symlinks are involved? Pilot error? Or should we
exclude them from textconv (and diff.*.command, for that matter)
at all times? See a test case below.
> Also, I'm happy to hear comments on which functionality should get
> text-converted. I think I like seeing the --stat report the binary
> changes.
At this time I also think that it is enough to apply textconv only
to generate diffs.
Test case follows:
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index e456746..dab4338 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -19,6 +19,18 @@ cat >expect.text <<'EOF'
+1
EOF
+cat >expect.typechange <<'EOF'
+Binary files a/file and /dev/null differ
+diff --git a/file b/file
+new file mode 120000
+index ad8b3d2..67be421
+--- /dev/null
++++ b/file
+@@ -0,0 +1 @@
++frotz
+\ No newline at end of file
+EOF
+
cat >hexdump <<'EOF'
#!/bin/sh
perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' "$1"
@@ -75,4 +87,16 @@ test_expect_success 'format-patch produces binary' '
test_cmp expect.binary actual
'
+# make a symlink the hard way that works on symlink-challenged file systems
+test_expect_failure 'textconv does not kick in if there is a type change' '
+ echo -n frotz > file &&
+ git add file &&
+ git ls-files -s | sed -e s/100644/120000/ |
+ git update-index --index-info &&
+ git commit -m typechange &&
+ git show >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.typechange actual
+'
+
test_done
next prev parent reply other threads:[~2008-10-24 7:17 UTC|newest]
Thread overview: 85+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-28 2:06 Implementation of a "textconv" filter for easy custom diff Matthieu Moy
2008-09-28 2:06 ` [PATCH] Facility to have multiple kinds of drivers for diff Matthieu Moy
2008-09-28 2:06 ` [PATCH] Implement run_command_to_buf (spawn a process and reads its stdout) Matthieu Moy
2008-09-28 2:06 ` [PATCH] Implement a textconv filter for "git diff" Matthieu Moy
2008-09-28 2:06 ` [PATCH] Document the textconv filter Matthieu Moy
2008-09-28 2:06 ` [PATCH] Add a basic test for " Matthieu Moy
2008-09-28 11:07 ` [PATCH] Document " Johannes Sixt
2008-09-28 12:29 ` Matthieu Moy
2008-09-28 4:15 ` [PATCH] Implement a textconv filter for "git diff" Jeff King
2008-09-28 10:00 ` Matthieu Moy
2008-09-28 16:12 ` Jeff King
2008-09-28 4:10 ` Implementation of a "textconv" filter for easy custom diff Jeff King
2008-09-28 9:57 ` Matthieu Moy
2008-09-28 16:11 ` Jeff King
2008-09-30 15:19 ` Matthieu Moy
2008-09-30 16:45 ` Jeff King
2008-10-05 21:41 ` [PATCH 0/4] diff text conversion filter Jeff King
2008-10-05 21:42 ` [PATCH 1/4] t4012: use test_cmp instead of cmp Jeff King
2008-10-05 21:43 ` [PATCH 2/4] diff: unify external diff and funcname parsing code Jeff King
2008-10-05 21:43 ` [PATCH 3/4] diff: introduce diff.<driver>.binary Jeff King
2008-10-07 15:17 ` Johannes Sixt
2008-10-07 15:35 ` Jeff King
2008-10-07 15:54 ` Johannes Sixt
2008-10-12 5:24 ` Junio C Hamano
2008-10-13 1:23 ` Jeff King
2008-10-13 4:00 ` Junio C Hamano
2008-10-13 4:15 ` Jeff King
2008-10-13 6:10 ` Johannes Sixt
2008-10-13 13:54 ` Junio C Hamano
2008-10-13 8:12 ` Matthieu Moy
2008-10-24 2:46 ` Jeff King
2008-10-24 2:48 ` [PATCH 1/5] diff: add missing static declaration Jeff King
2008-10-24 2:50 ` [PATCH 2/5] add userdiff textconv tests Jeff King
2008-10-24 2:53 ` [PATCH 3/5] refactor userdiff textconv code Jeff King
2008-10-24 7:15 ` Johannes Sixt [this message]
2008-10-24 12:40 ` Jeff King
2008-10-24 13:51 ` Jeff King
2008-10-24 14:01 ` Johannes Sixt
2008-10-24 14:08 ` Jeff King
2008-10-24 21:12 ` Junio C Hamano
2008-10-24 22:50 ` Jeff King
2008-10-24 22:56 ` Jeff King
2008-10-25 0:48 ` Jeff King
2008-10-25 0:50 ` [PATCH 1/7] diff: add missing static declaration Jeff King
2008-10-25 0:51 ` [PATCH 2/7] add userdiff textconv tests Jeff King
2008-10-25 0:52 ` [PATCH 3/7] textconv: assume text-converted contents are not binary Jeff King
2008-10-25 0:52 ` [PATCH 4/7] textconv: don't convert for every operation Jeff King
2008-10-25 5:41 ` Junio C Hamano
2008-10-25 7:19 ` Jeff King
2008-10-25 18:32 ` Junio C Hamano
2008-10-25 19:35 ` Jeff King
2008-10-25 23:35 ` Junio C Hamano
2008-10-25 23:48 ` Junio C Hamano
2008-10-26 4:52 ` Jeff King
2008-10-26 4:38 ` Jeff King
2008-10-26 4:41 ` [PATCH v3 1/8] diff: add missing static declaration Jeff King
2008-10-26 4:41 ` [PATCH v3 2/8] document the diff driver textconv feature Jeff King
2008-10-26 4:42 ` [PATCH v3 3/8] add userdiff textconv tests Jeff King
2008-10-26 4:44 ` [PATCH v3 4/8] refactor userdiff textconv code Jeff King
2008-10-26 4:45 ` [PATCH v3 5/8] userdiff: require explicitly allowing textconv Jeff King
2008-10-26 4:46 ` [PATCH v3 6/8] only textconv regular files Jeff King
2008-10-26 4:49 ` [PATCH v3 7/8] wt-status: load diff ui config Jeff King
2008-10-27 5:30 ` Junio C Hamano
2008-10-27 8:23 ` Jeff King
2008-10-26 4:50 ` [PATCH v3 8/8] enable textconv for diff in verbose status/commit Jeff King
2008-10-25 0:54 ` [PATCH 5/7] userdiff: require explicitly allowing textconv Jeff King
2008-10-25 0:54 ` [PATCH 6/7] document the diff driver textconv feature Jeff King
2008-10-25 0:55 ` [PATCH 7/7] only textconv regular files Jeff King
2008-10-24 2:55 ` [PATCH 4/5] userdiff: require explicitly allowing textconv Jeff King
2008-10-24 7:04 ` Johannes Sixt
2008-10-24 2:56 ` [PATCH 5/5] document the diff driver textconv feature Jeff King
2008-10-24 7:02 ` [PATCH 3/4] diff: introduce diff.<driver>.binary Johannes Sixt
2008-10-05 21:43 ` [PATCH 4/4] diff: add filter for converting binary to text Jeff King
2008-10-05 22:03 ` [PATCH 0/4] diff text conversion filter Jakub Narebski
2008-10-06 6:29 ` Johannes Sixt
2008-10-06 6:52 ` Jeff King
2008-10-06 8:55 ` Johannes Sixt
2008-10-06 15:15 ` Matthieu Moy
2008-10-07 1:20 ` Jeff King
2008-10-07 5:52 ` Johannes Sixt
2008-10-07 6:00 ` Jeff King
2008-10-07 6:15 ` Matthieu Moy
2008-10-07 15:46 ` Jeff King
2008-10-07 16:15 ` Johannes Sixt
2008-10-13 1:29 ` Jeff King
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=4901762A.3090003@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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).