On Tue, Apr 12, 2011 at 11:04:43AM +0200, Michael J Gruber wrote: > > This works fine, except for merge commits. > > For merge commits i see the diff of the binary contents of the file. > > > > Is this intentional? > > git help gitattributes mentions no such limitation. > > Anywhere else(gitk(on non merge commit), git gui blame) i see the the filtered textual representation of the file. > > > > I tried 1.7.4 msysgit and current master > > > > Greetings Peter > > textconv is applied for "diff -m" but not for combined diffs (-c, --cc) > at the moment. They go through a completely different codepath, so it is > expected code-wise (not a bug per se) but not ui-wise. > > Looking at the code and trying to dig something up atm... Ick. I started with this test: diff --git a/t/t4046-diff-textconv-merge.sh b/t/t4046-diff-textconv-merge.sh new file mode 100755 index 0000000..8643330 --- /dev/null +++ b/t/t4046-diff-textconv-merge.sh @@ -0,0 +1,35 @@ +#!/bin/sh + +test_description='combined diff uses textconv' +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit one file && + test_commit two file && + git checkout -b other HEAD^ && + test_commit three file && + test_must_fail git merge master && + echo resolved >file && + git commit -a && + echo "file diff=upcase" >.gitattributes && + git config diff.upcase.textconv "tr a-z A-Z <" +' + +cat >expect <<'EOF' +Merge branch 'master' into other + +diff --combined file +index 2bdf67a,f719efd..2ab19ae +--- a/file ++++ b/file +@@@ -1,1 -1,1 +1,1 @@@ +- THREE + -TWO +++RESOLVED +EOF +test_expect_success 'diff -c uses textconv' ' + git show --format=%s -c >actual && + test_cmp expect actual +' + +test_done but after looking at the codepath, it is must worse than just textconv. Try this: git init repo && cd repo && openssl rand 64 >file.bin && git add file.bin && git commit -m one && openssl rand 64 >file.bin && git commit -a -m two && git checkout -b other HEAD^ openssl rand 64 >file.bin && git commit -a -m three && (git merge master || true) && openssl rand 64 >file.bin && git commit -a -m resolved && git show We just dump the binary goo all over the terminal. So I think the whole combined-diff code path needs to learn how to handle binaries properly. Unfortunately, it seems to be totally distinct from the regular diff code path. It doesn't even use diff_filespecs, so our usual is_binary and textconv code won't work. So the best way forward may involve significant refactoring. And of course we have to figure out sane semantics. The textconv case is easy; just use the textconv blobs instead of the regular ones. But what should the true binary case (as in the rand example above) show? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html