Hi, Two quick thoughts: Jeff King wrote: > The C mappings are still here, but see the next patch. This is adding a regression in order to remove it. I guess it's harmless, but I don't see the point. [...] > --- a/t/t4012-diff-binary.sh > +++ b/t/t4012-diff-binary.sh > @@ -90,4 +90,17 @@ test_expect_success 'diff --no-index with binary creation' ' > test_cmp expected actual > ' > > +test_expect_success 'binary files are not considered text by file extension' ' > + echo Q | q_to_nul >binary.c && > + git add binary.c && > + cat >expect <<-\EOF && > + diff --git a/binary.c b/binary.c > + new file mode 100644 > + index 0000000..1f2a4f5 > + Binary files /dev/null and b/binary.c differ > + EOF > + git diff --cached binary.c >actual && > + test_cmp expect actual Re the idea of this test: very good idea. Re the mechanics: I would have been happier to see echo Q | q_to_nul >binary.c && git add binary.c && git diff --cached binary.c >diff && grep Binary files diff since that would avoid hard-coding some assumptions: - the blob name of binary.c - that [diff] mnemonicprefix defaults to false (I'd like to see the default change to true) - that [core] abbrev defaults to 7 (it probably won't change, but it's a distracting detail, and if we were starting over 8 might be a better default) A bonus comment: :) [...] > --- a/t/t4018-diff-funcname.sh > +++ b/t/t4018-diff-funcname.sh > @@ -124,7 +124,9 @@ do > done > > test_expect_success 'default behaviour' ' > - rm -f .gitattributes && > + cat >.gitattributes <<-\EOF && > + *.java diff=default > + EOF > test_expect_funcname "public class Beer\$" > ' echo "*.java diff=default" >.gitattributes would do the same with two lines fewer. :) Thanks for working on this. I owe you a beer. Jonathan -- 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