On Mon, Dec 19, 2011 at 12:07:33PM -0600, Jonathan Nieder 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. It's purely an attempt to help somebody reading "git log" later understand what happened. Maybe a comment in the commit message is more appropriate. > > +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 Yeah, I think that's fine, and I'll squash it in to my local version. It does miss one problem, though (that is also present in my original): using "binary.c" is no longer a good name, since the next patch will revert the "*.c" bits. :) > > --- 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. :) Yup. I was following the style of the test directly below, which sets both java and perl drivers. But the "default" test that needed updating only checks the java case. Will squash. > Thanks for working on this. I owe you a beer. You're welcome. :) -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