{"thread":{"id":"27361","subject":"Problems with Git's \"perl\" userdiff driver","startedAt":"2011-05-15T18:14:53Z","lastAt":"2011-05-22T17:29:32Z","messageCount":13,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"167902","messageId":"BANLkTi=OXznTspN-CJjM0YXfqARxL=J+Ow@mail.gmail.com","threadId":"27361","inReplyTo":null,"subject":"Problems with Git's \"perl\" userdiff driver","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-05-15T18:14:53Z","receivedAt":"2011-05-15T18:14:53Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Dec 29, 2010 at 06:22, Junio C Hamano wrote:\n\n[better late than never]\n\n> * jn/perl-funcname (2010-12-27) 2 commits\n>  - userdiff/perl: catch BEGIN/END/... and POD as headers\n>  - diff: funcname and word patterns for perl\n\nThe POD rule doesn't work properly. I suspect it has to be:\n\n \"^=head[0-9] .*\",\n\nInstead of the current:\n\n \"^=head[0-9] \",\n\nSince e.g.:\n\n =head1 WHATEVER\n\nWill just be shown as:\n\n =head1\n\nIn the diff context.\n\nAnd actually it applies very badly to POD in general, since the \"sub\"\nrule will be tried first, so e.g. in Perldoc we'll often end up\nfinding some \"sub\" example halfway up the file, instead of the =head1*\nor =item* section a few lines up.\n\nAnd it looks like the regex only catches:\n\n sub foo {\n }\n\nNot:\n\n sub foo\n {\n }\n"},{"id":"167913","messageId":"7vk4drpwal.fsf@alter.siamese.dyndns.org","threadId":"27361","inReplyTo":"BANLkTi=OXznTspN-CJjM0YXfqARxL=J+Ow@mail.gmail.com","subject":"Re: Problems with Git's \"perl\" userdiff driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T20:02:10Z","receivedAt":"2011-05-15T20:02:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Dec 29, 2010 at 06:22, Junio C Hamano wrote:\n\nHeh; without in-reply-to or references, this is useless, isn't it?\n\n> [better late than never]\n>\n>> * jn/perl-funcname (2010-12-27) 2 commits\n>> - userdiff/perl: catch BEGIN/END/... and POD as headers\n>> - diff: funcname and word patterns for perl\n\nFor others, the above are these two:\n\n a25e473 (userdiff/perl: catch BEGIN/END/... and POD as headers, 2010-12-27)\n 71a5d4b (diff: funcname and word patterns for perl, 2010-12-26)\n\n> The POD rule doesn't work properly. I suspect it has to be:\n>\n> \"^=head[0-9] .*\",\n>\n> Instead of the current:\n>\n> \"^=head[0-9] \",\n>\n> Since e.g.:\n>\n> =head1 WHATEVER\n>\n> Will just be shown as:\n>\n> =head1\n>\n> In the diff context.\n>\n> And actually it applies very badly to POD in general, since the \"sub\"\n> rule will be tried first, so e.g. in Perldoc we'll often end up\n> finding some \"sub\" example halfway up the file, instead of the =head1*\n> or =item* section a few lines up.\n>\n> And it looks like the regex only catches:\n>\n> sub foo {\n> }\n>\n> Not:\n>\n> sub foo\n> {\n> }\n"},{"id":"167915","messageId":"BANLkTik_Y0uXeO5kd7jxK=4Q47uickEVoQ@mail.gmail.com","threadId":"27361","inReplyTo":"7vk4drpwal.fsf@alter.siamese.dyndns.org","subject":"Re: Problems with Git's \"perl\" userdiff driver","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-05-15T20:13:02Z","receivedAt":"2011-05-15T20:13:02Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, May 15, 2011 at 22:02, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Wed, Dec 29, 2010 at 06:22, Junio C Hamano wrote:\n>\n> Heh; without in-reply-to or references, this is useless, isn't it?\n\nAs far as I could tell the patch you appended to that series was never\nsubmitted to the list, so I couldn't reply to it. Thanks for providing\nthe sha1's.\n"},{"id":"168422","messageId":"20110521185314.GA10530@elie","threadId":"27361","inReplyTo":"BANLkTi=OXznTspN-CJjM0YXfqARxL=J+Ow@mail.gmail.com","subject":"[PATCH 0/7] Re: Problems with Git's \"perl\" userdiff driver","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T18:53:14Z","receivedAt":"2011-05-21T18:53:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ævar,\n\nÆvar Arnfjörð Bjarmason wrote:\n\n> The POD rule doesn't work properly. I suspect it has to be:\n\nThanks. Some context is here (thanks to Thomas for his notes/full\nbranch).\n\n http://thread.gmane.org/gmane.comp.version-control.git/164184\n\nQuick thoughts before fixing them:\n\n> \"^=head[0-9] .*\",\n>\n> Instead of the current:\n>\n> \"^=head[0-9] \",\n\nGood catch.\n\n> And actually it applies very badly to POD in general, since the \"sub\"\n> rule will be tried first, so e.g. in Perldoc we'll often end up\n> finding some \"sub\" example halfway up the file, instead of the =head1*\n> or =item* section a few lines up.\n\nI have another worry of the same kind. If my source file consists of\nmultiple packages (like git-svn.perl does), then the \"package\" rule\ncan be useful for getting bearings in a diff:\n\n\tdiff --git a/git-svn.perl b/git-svn.perl\n\tindex dc66803..74d8612 100755\n\t--- a/git-svn.perl\n\t+++ b/git-svn.perl\n\t@@ -4000,7 +4000,6 @@ package SVN::Git::Fetcher;\n\t use strict;\n\t use warnings;\n\t use Carp qw/croak/;\n\t-use File::Temp qw/tempfile/;\n\t use IO::File qw//;\n\t use vars qw/$_ignore_regex/;\n\nBut as soon as \"git diff\" encounters a subroutine, that replaces the\ncontext. It seems that the --show-function mechanism works best for\nfiles structured such that every line has a scope, with no nesting:\n\n\tsub foo {\n\t\t... lines of foo come here ...\n\t}\n\n\tsub bar {\n\t\t... lines of bar come here ...\n\t}\n\nand does not cope as well with nested scopes or lines that do not\nfall in any scope.\n\nThe upshot is that all these patterns should be anchored at the left\nmargin for now, and it might be worth considering teaching userdiff to\npush and pop scopes so that lines outside the innermost scope are\ncorrectly attributed, as in the last two lines below:\n\n\tpackage Foo::Bar::Baz\n\n\t=head1 NAME\n\n\tFoo::Bar::Baz - well-documented frobber\n\n\t=cut\n\n\tuse strict;\n\tuse Qux::Quux;\n\nThat would also take care of your example of\n\n\t=item B\n\n\tUse it like so:\n\n\t\tsub example {\n\t\t\tsomething else;\n\t\t}\n\n\t... more documentation that should be attributed to the =item,\n\tnot the sub ...\n\n> And it looks like the regex only catches:\n>\n> sub foo {\n> }\n>\n> Not:\n>\n> sub foo\n> {\n> }\n\nYep, that seems worth fixing, too. Patches follow, with a few unrelated\ncleanups thrown in. These patches are based against \"maint\" for no\nparticular reason. Bugs and improvements welcome, of course. :)\n\nJonathan Nieder (7):\n t4018 (diff funcname patterns): make .gitattributes state easier to\n track\n t4018 (diff funcname patterns): make configuration easier to track\n t4018 (diff funcname patterns): minor tweaks\n userdiff/perl: anchor \"sub\" and \"package\" patterns on the left\n userdiff/perl: match full line of POD headers\n userdiff/perl: catch subs with brace on second line\n tests: make test_expect_code quieter on success\n\n t/t4018-diff-funcname.sh | 148 ++++++++++++++++++++++++++++++++++++---------\n t/test-lib.sh | 7 +-\n userdiff.c | 22 ++++++-\n 3 files changed, 139 insertions(+), 38 deletions(-)\n"},{"id":"168424","messageId":"20110521191133.GB10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 1/7] t4018 (funcname patterns): make .gitattributes state easier to track","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:11:33Z","receivedAt":"2011-05-21T19:11:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Most, but not all, tests in this script rely on attributes declaring\nthat files with a .java extension should use the \"java\" driver:\n\n\t*.java diff=java\n\nSplit out a \"set up\" test to put such a .gitattributes in place after\nthe tests that do not want it have run, to make it more likely that\nindividual tests other than this setup test can be safely modified,\nrearranged, or skipped. Presumably this setup code will learn to\nrequest other drivers for other extensions in the same place when the\ntest suite learns to exercise other diff drivers.\n\nSimilarly, make sure that early test assertions that do not use these\ndefault attributes set up .gitattributes appropriately for themselves,\nso tests that run before can be modified with less risk of breaking\nsomething.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t4018-diff-funcname.sh | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 3646930..24eb1a3 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -36,11 +36,12 @@ builtin_patterns=\"bibtex cpp csharp fortran html java objc pascal perl php pytho\n for p in $builtin_patterns\n do\n \ttest_expect_success \"builtin $p pattern compiles\" '\n-\t\techo \"*.java diff=$p\" > .gitattributes &&\n+\t\techo \"*.java diff=$p\" >.gitattributes &&\n \t\t! { git diff --no-index Beer.java Beer-correct.java 2>&1 |\n \t\t\tgrep \"fatal\" > /dev/null; }\n \t'\n \ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n+\t\techo \"*.java diff=$p\" >.gitattributes &&\n \t\t! { git diff --no-index --word-diff \\\n \t\t\tBeer.java Beer-correct.java 2>&1 |\n \t\t\tgrep \"fatal\" > /dev/null; }\n@@ -53,8 +54,11 @@ test_expect_success 'default behaviour' '\n \tgrep \"^@@.*@@ public class Beer\"\n '\n \n+test_expect_success 'set up .gitattributes declaring drivers to test' '\n+\techo \"*.java diff=java\" >.gitattributes\n+'\n+\n test_expect_success 'preset java pattern' '\n-\techo \"*.java diff=java\" >.gitattributes &&\n \tgit diff --no-index Beer.java Beer-correct.java |\n \tgrep \"^@@.*@@ public static void main(\"\n '\n-- \n1.7.5.1\n"},{"id":"168425","messageId":"20110521192228.GC10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 2/7] t4018 (funcname patterns): make configuration easier to track","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:22:28Z","receivedAt":"2011-05-21T19:22:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Introduce a \"test_config\" function to set a configuration variable\nfor use by a single test (automatically unsetting it when the\nassertion finishes). If this function is used consistently, the\nconfiguration used in a test_expect_success block can be read at the\nbeginning of that block instead of requiring reading all the tests\nthat come before. So it becomes a little easier to add new tests or\nrearrange existing ones without fear of breaking configuration.\n\nIn particular, the test of alternation in xfuncname patterns also\nchecks that xfuncname takes precedence over funcname variable as a\nsort of side-effect, since the latter leaks in from previous tests.\nIn the new syntax, the test has to say explicitly what variables it is\nusing, making the test clearer and a future regression in coverage\nfrom carelessly editing the script less likely.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t4018-diff-funcname.sh | 19 ++++++++++++-------\n 1 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 24eb1a3..ce0a0e3 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -32,6 +32,11 @@ EOF\n \n sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n \n+test_config () {\n+\tgit config \"$1\" \"$2\" &&\n+\ttest_when_finished \"git config --unset $1\"\n+}\n+\n builtin_patterns=\"bibtex cpp csharp fortran html java objc pascal perl php python ruby tex\"\n for p in $builtin_patterns\n do\n@@ -63,29 +68,29 @@ test_expect_success 'preset java pattern' '\n \tgrep \"^@@.*@@ public static void main(\"\n '\n \n-git config diff.java.funcname '!static\n-!String\n-[^ \t].*s.*'\n-\n test_expect_success 'custom pattern' '\n+\ttest_config diff.java.funcname \"!static\n+!String\n+[^ \t].*s.*\" &&\n \tgit diff --no-index Beer.java Beer-correct.java |\n \tgrep \"^@@.*@@ int special;$\"\n '\n \n test_expect_success 'last regexp must not be negated' '\n-\tgit config diff.java.funcname \"!static\" &&\n+\ttest_config diff.java.funcname \"!static\" &&\n \tgit diff --no-index Beer.java Beer-correct.java 2>&1 |\n \tgrep \"fatal: Last expression must not be negated:\"\n '\n \n test_expect_success 'pattern which matches to end of line' '\n-\tgit config diff.java.funcname \"Beer$\" &&\n+\ttest_config diff.java.funcname \"Beer$\" &&\n \tgit diff --no-index Beer.java Beer-correct.java |\n \tgrep \"^@@.*@@ Beer\"\n '\n \n test_expect_success 'alternation in pattern' '\n-\tgit config diff.java.xfuncname \"^[ \t]*((public|static).*)$\" &&\n+\ttest_config diff.java.funcname \"Beer$\" &&\n+\ttest_config diff.java.xfuncname \"^[ \t]*((public|static).*)$\" &&\n \tgit diff --no-index Beer.java Beer-correct.java |\n \tgrep \"^@@.*@@ public static void main(\"\n '\n-- \n1.7.5.1\n"},{"id":"168426","messageId":"20110521192514.GD10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 3/7] t4018 (funcname patterns): minor cleanups","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:25:14Z","receivedAt":"2011-05-21T19:25:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Introduce a test_expect_funcname function to make a diff and apply a\nregexp anchored on the left to the function name it writes, avoiding\nsome repetition.\n\nOmit the space after >, <<, and < operators for consistency with\nother scripts. Quote the <\n---\n t/t4018-diff-funcname.sh | 49 +++++++++++++++++++++++----------------------\n 1 files changed, 25 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex ce0a0e3..ad74c60 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -9,8 +9,7 @@ test_description='Test custom diff function name patterns'\n \n LF='\n '\n-\n-cat > Beer.java << EOF\n+cat >Beer.java <<\\EOF\n public class Beer\n {\n \tint special;\n@@ -29,34 +28,40 @@ public class Beer\n \t}\n }\n EOF\n-\n-sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n+sed 's/beer\\\\/beer,\\\\/' Beer-correct.java\n \n test_config () {\n \tgit config \"$1\" \"$2\" &&\n \ttest_when_finished \"git config --unset $1\"\n }\n \n-builtin_patterns=\"bibtex cpp csharp fortran html java objc pascal perl php python ruby tex\"\n-for p in $builtin_patterns\n+test_expect_funcname () {\n+\ttest_expect_code 1 git diff --no-index \\\n+\t\tBeer.java Beer-correct.java >diff &&\n+\tgrep \"^@@.*@@ $1\" diff\n+}\n+\n+for p in bibtex cpp csharp fortran html java objc pascal perl php python ruby tex\n do\n \ttest_expect_success \"builtin $p pattern compiles\" '\n \t\techo \"*.java diff=$p\" >.gitattributes &&\n-\t\t! { git diff --no-index Beer.java Beer-correct.java 2>&1 |\n-\t\t\tgrep \"fatal\" > /dev/null; }\n+\t\ttest_expect_code 1 git diff --no-index \\\n+\t\t\tBeer.java Beer-correct.java 2>msg &&\n+\t\t! grep fatal msg &&\n+\t\t! grep error msg\n \t'\n \ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n \t\techo \"*.java diff=$p\" >.gitattributes &&\n-\t\t! { git diff --no-index --word-diff \\\n-\t\t\tBeer.java Beer-correct.java 2>&1 |\n-\t\t\tgrep \"fatal\" > /dev/null; }\n+\t\ttest_expect_code 1 git diff --no-index --word-diff \\\n+\t\t\tBeer.java Beer-correct.java 2>msg &&\n+\t\t! grep fatal msg &&\n+\t\t! grep error msg\n \t'\n done\n \n test_expect_success 'default behaviour' '\n \trm -f .gitattributes &&\n-\tgit diff --no-index Beer.java Beer-correct.java |\n-\tgrep \"^@@.*@@ public class Beer\"\n+\ttest_expect_funcname \"public class Beer\\$\"\n '\n \n test_expect_success 'set up .gitattributes declaring drivers to test' '\n@@ -64,35 +69,31 @@ test_expect_success 'set up .gitattributes declaring drivers to test' '\n '\n \n test_expect_success 'preset java pattern' '\n-\tgit diff --no-index Beer.java Beer-correct.java |\n-\tgrep \"^@@.*@@ public static void main(\"\n+\ttest_expect_funcname \"public static void main(\"\n '\n \n test_expect_success 'custom pattern' '\n \ttest_config diff.java.funcname \"!static\n !String\n [^ \t].*s.*\" &&\n-\tgit diff --no-index Beer.java Beer-correct.java |\n-\tgrep \"^@@.*@@ int special;$\"\n+\ttest_expect_funcname \"int special;\\$\"\n '\n \n test_expect_success 'last regexp must not be negated' '\n \ttest_config diff.java.funcname \"!static\" &&\n-\tgit diff --no-index Beer.java Beer-correct.java 2>&1 |\n-\tgrep \"fatal: Last expression must not be negated:\"\n+\ttest_expect_code 128 git diff --no-index Beer.java Beer-correct.java 2>msg &&\n+\tgrep \": Last expression must not be negated:\" msg\n '\n \n test_expect_success 'pattern which matches to end of line' '\n-\ttest_config diff.java.funcname \"Beer$\" &&\n-\tgit diff --no-index Beer.java Beer-correct.java |\n-\tgrep \"^@@.*@@ Beer\"\n+\ttest_config diff.java.funcname \"Beer\\$\" &&\n+\ttest_expect_funcname \"Beer\\$\"\n '\n \n test_expect_success 'alternation in pattern' '\n \ttest_config diff.java.funcname \"Beer$\" &&\n \ttest_config diff.java.xfuncname \"^[ \t]*((public|static).*)$\" &&\n-\tgit diff --no-index Beer.java Beer-correct.java |\n-\tgrep \"^@@.*@@ public static void main(\"\n+\ttest_expect_funcname \"public static void main(\"\n '\n \n test_done\n-- \n1.7.5.1\n"},{"id":"168427","messageId":"20110521192901.GE10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 4/7] userdiff/perl: anchor \"sub\" and \"package\" patterns on the left","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:29:01Z","receivedAt":"2011-05-21T19:29:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The userdiff funcname mechanism has no concept of nested scopes ---\ninstead, \"git diff\" and \"git grep --show-function\" simply label the\ndiff header with the most recent matching line. Unfortunately that\nmeans text following a subroutine in a POD section:\n\n\t=head1 DESCRIPTION\n\n\tYou might use this facility like so:\n\n\t\tsub example {\n\t\t\tfoo;\n\t\t}\n\n\tNow, having said that, let's say more about the facility.\n\tBlah blah blah ... etc etc.\n\ngets the subroutine name instead of the POD header in its diff/grep\nfuncname header, making it harder to get oriented when reading a\ndiff without enough context.\n\nThe fix is simple: anchor the funcname syntax to the left margin so\nnested subroutines and packages like this won't get picked up. (The\nbuiltin C++ funcname pattern already does the same thing.) This means\nthe userdiff driver will misparse the idiom\n\n\t{\n\t\tmy $static;\n\t\tsub foo {\n\t\t\t... use $static ...\n\t\t}\n\t}\n\nbut I think that's worth it; we can revisit this later if the userdiff\nmechanism learns to keep track of the beginning and end of nested\nscopes.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jonathan Nieder \n---\n t/t4018-diff-funcname.sh | 59 +++++++++++++++++++++++++++++++++++++++++++--\n userdiff.c | 4 +-\n 2 files changed, 58 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex ad74c60..f071a8f 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -29,6 +29,47 @@ public class Beer\n }\n EOF\n sed 's/beer\\\\/beer,\\\\/' Beer-correct.java\n+cat >Beer.perl <<\\EOF\n+package Beer;\n+\n+use strict;\n+use warnings;\n+use parent qw(Exporter);\n+our @EXPORT_OK = qw(round);\n+\n+sub round {\n+\tmy ($n) = @_;\n+\tprint \"$n bottles of beer on the wall \";\n+\tprint \"$n bottles of beer\\n\";\n+\tprint \"Take one down, pass it around, \";\n+\t$n = $n - 1;\n+\tprint \"$n bottles of beer on the wall.\\n\";\n+}\n+\n+__END__\n+\n+=head1 NAME\n+\n+Beer - subroutine to output fragment of a drinking song\n+\n+=head1 SYNOPSIS\n+\n+\tuse Beer qw(round);\n+\n+\tsub song {\n+\t\tfor (my $i = 99; $i > 0; $i--) {\n+\t\t\tround $i;\n+\t\t}\n+\t}\n+\n+\tsong;\n+\n+=cut\n+EOF\n+sed -e '\n+\ts/beer\\\\/beer,\\\\/\n+\ts/song;/song();/\n+' Beer-correct.perl\n \n test_config () {\n \tgit config \"$1\" \"$2\" &&\n@@ -36,8 +77,9 @@ test_config () {\n }\n \n test_expect_funcname () {\n-\ttest_expect_code 1 git diff --no-index \\\n-\t\tBeer.java Beer-correct.java >diff &&\n+\tlang=${2-java}\n+\ttest_expect_code 1 git diff --no-index -U1 \\\n+\t\t\"Beer.$lang\" \"Beer-correct.$lang\" >diff &&\n \tgrep \"^@@.*@@ $1\" diff\n }\n \n@@ -65,13 +107,24 @@ test_expect_success 'default behaviour' '\n '\n \n test_expect_success 'set up .gitattributes declaring drivers to test' '\n-\techo \"*.java diff=java\" >.gitattributes\n+\tcat >.gitattributes <<-\\EOF\n+\t*.java diff=java\n+\t*.perl diff=perl\n+\tEOF\n '\n \n test_expect_success 'preset java pattern' '\n \ttest_expect_funcname \"public static void main(\"\n '\n \n+test_expect_success 'preset perl pattern' '\n+\ttest_expect_funcname \"sub round {\\$\" perl\n+'\n+\n+test_expect_success 'perl pattern is not distracted by sub within POD' '\n+\ttest_expect_funcname \"=head\" perl\n+'\n+\n test_expect_success 'custom pattern' '\n \ttest_config diff.java.funcname \"!static\n !String\ndiff --git a/userdiff.c b/userdiff.c\nindex 1ff4797..2cca0af 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -60,8 +60,8 @@ PATTERNS(\"pascal\",\n \t \"|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+\"\n \t \"|<>|<=|>=|:=|\\\\.\\\\.\"),\n PATTERNS(\"perl\",\n-\t \"^[ \\t]*package .*;\\n\"\n-\t \"^[ \\t]*sub .* \\\\{\\n\"\n+\t \"^package .*;\\n\"\n+\t \"^sub .* \\\\{\\n\"\n \t \"^[A-Z]+ \\\\{\\n\"\t/* BEGIN, END, ... */\n \t \"^=head[0-9] \",\t/* POD */\n \t /* -- */\n-- \n1.7.5.1\n"},{"id":"168428","messageId":"20110521193551.GF10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 5/7] userdiff/perl: match full line of POD headers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:35:51Z","receivedAt":"2011-05-21T19:35:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The builtin perl userdiff driver is not greedy enough about catching\nPOD header lines. Capture the whole line, so instead of just\ndeclaring that we are in some \"@@ =head1\" section, diff/grep output\ncan explain that the enclosing section is about \"@@ =head1 OPTIONS\".\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jonathan Nieder \n---\n t/t4018-diff-funcname.sh | 4 ++++\n userdiff.c | 2 +-\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex f071a8f..8a57149 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -125,6 +125,10 @@ test_expect_success 'perl pattern is not distracted by sub within POD' '\n \ttest_expect_funcname \"=head\" perl\n '\n \n+test_expect_success 'perl pattern gets full line of POD header' '\n+\ttest_expect_funcname \"=head1 SYNOPSIS\\$\" perl\n+'\n+\n test_expect_success 'custom pattern' '\n \ttest_config diff.java.funcname \"!static\n !String\ndiff --git a/userdiff.c b/userdiff.c\nindex 2cca0af..32ead96 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -63,7 +63,7 @@ PATTERNS(\"perl\",\n \t \"^package .*;\\n\"\n \t \"^sub .* \\\\{\\n\"\n \t \"^[A-Z]+ \\\\{\\n\"\t/* BEGIN, END, ... */\n-\t \"^=head[0-9] \",\t/* POD */\n+\t \"^=head[0-9] .*\",\t/* POD */\n \t /* -- */\n \t \"[[:alpha:]_'][[:alnum:]_']*\"\n \t \"|0[xb]?[0-9a-fA-F_]*\"\n-- \n1.7.5.1\n"},{"id":"168429","messageId":"20110521193826.GG10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 6/7] userdiff/perl: catch sub with brace on second line","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:38:26Z","receivedAt":"2011-05-21T19:38:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Accept\n\n\tsub foo\n\t{\n\t}\n\nas an alternative to a more common style that introduces perl\nfunctions with a brace on the first line (and likewise for BEGIN/END\nblocks). The new regex is a little hairy to avoid matching\n\n\t# forward declaration\n\tsub foo;\n\nwhile continuing to match \"sub foo($;@) {\" and\n\n\tsub foo { # This routine is interesting;\n\t\t# in fact, the lines below explain how...\n\nWhile at it, pay attention to Perl 5.14's \"package foo {\" syntax as an\nalternative to the traditional \"package foo;\".\n\nRequested-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Jonathan Nieder \n---\n t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--\n userdiff.c | 20 +++++++++++++++++---\n 2 files changed, 40 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 8a57149..b2fd1a9 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -35,7 +35,11 @@ package Beer;\n use strict;\n use warnings;\n use parent qw(Exporter);\n-our @EXPORT_OK = qw(round);\n+our @EXPORT_OK = qw(round finalround);\n+\n+sub other; # forward declaration\n+\n+# hello\n \n sub round {\n \tmy ($n) = @_;\n@@ -46,6 +50,12 @@ sub round {\n \tprint \"$n bottles of beer on the wall.\\n\";\n }\n \n+sub finalround\n+{\n+\tprint \"Go to the store, buy some more\\n\";\n+\tprint \"99 bottles of beer on the wall.\\n\");\n+}\n+\n __END__\n \n =head1 NAME\n@@ -54,12 +64,13 @@ Beer - subroutine to output fragment of a drinking song\n \n =head1 SYNOPSIS\n \n-\tuse Beer qw(round);\n+\tuse Beer qw(round finalround);\n \n \tsub song {\n \t\tfor (my $i = 99; $i > 0; $i--) {\n \t\t\tround $i;\n \t\t}\n+\t\tfinalround;\n \t}\n \n \tsong;\n@@ -67,7 +78,9 @@ Beer - subroutine to output fragment of a drinking song\n =cut\n EOF\n sed -e '\n+\ts/hello/goodbye/\n \ts/beer\\\\/beer,\\\\/\n+\ts/more\\\\/more,\\\\/\n \ts/song;/song();/\n ' Beer-correct.perl\n \n@@ -121,6 +134,10 @@ test_expect_success 'preset perl pattern' '\n \ttest_expect_funcname \"sub round {\\$\" perl\n '\n \n+test_expect_success 'perl pattern accepts K&R style brace placement, too' '\n+\ttest_expect_funcname \"sub finalround\\$\" perl\n+'\n+\n test_expect_success 'perl pattern is not distracted by sub within POD' '\n \ttest_expect_funcname \"=head\" perl\n '\n@@ -129,6 +146,10 @@ test_expect_success 'perl pattern gets full line of POD header' '\n \ttest_expect_funcname \"=head1 SYNOPSIS\\$\" perl\n '\n \n+test_expect_success 'perl pattern is not distracted by forward declaration' '\n+\ttest_expect_funcname \"package Beer;\\$\" perl\n+'\n+\n test_expect_success 'custom pattern' '\n \ttest_config diff.java.funcname \"!static\n !String\ndiff --git a/userdiff.c b/userdiff.c\nindex 32ead96..42b86ac 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -60,9 +60,23 @@ PATTERNS(\"pascal\",\n \t \"|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+\"\n \t \"|<>|<=|>=|:=|\\\\.\\\\.\"),\n PATTERNS(\"perl\",\n-\t \"^package .*;\\n\"\n-\t \"^sub .* \\\\{\\n\"\n-\t \"^[A-Z]+ \\\\{\\n\"\t/* BEGIN, END, ... */\n+\t \"^package .*\\n\"\n+\t \"^sub [[:alnum:]_':]+[ \\t]*\"\n+\t\t\"(\\\\([^)]*\\\\)[ \\t]*)?\" /* prototype */\n+\t\t/*\n+\t\t * Attributes. A regex can't count nested parentheses,\n+\t\t * so just slurp up whatever we see, taking care not\n+\t\t * to accept lines like \"sub foo; # defined elsewhere\".\n+\t\t *\n+\t\t * An attribute could contain a semicolon, but at that\n+\t\t * point it seems reasonable enough to give up.\n+\t\t */\n+\t\t\"(:[^;#]*)?\"\n+\t\t\"(\\\\{[ \\t]*)?\" /* brace can come here or on the next line */\n+\t\t\"(#.*)?$\\n\" /* comment */\n+\t \"^[A-Z]+[ \\t]*\"\t/* BEGIN, END, ... */\n+\t\t\"(\\\\{[ \\t]*)?\" /* brace can come here or on the next line */\n+\t\t\"(#.*)?$\\n\"\n \t \"^=head[0-9] .*\",\t/* POD */\n \t /* -- */\n \t \"[[:alpha:]_'][[:alnum:]_']*\"\n-- \n1.7.5.1\n"},{"id":"168430","messageId":"20110521194032.GH10530@elie","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"[PATCH 7/7] tests: make test_expect_code quieter on success","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-21T19:40:32Z","receivedAt":"2011-05-21T19:40:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A command exiting with the expected status is not particularly\nnotable.\n\nWhile the indication of progress might be useful when tracking down\nwhere in a test a failure has happened, the same applies to most other\ntest helpers, which are quiet about success, so this single helper's\noutput stands out in an unpleasant way. An alternative method for\nshowing progress information might to invent a --progress option that\nruns tests with \"set -x\", or until that is available, to run tests\nusing commands like\n\n\tprove -v -j2 --shuffle --exec='sh -x' t2202-add-addremove.sh\n\nSigned-off-by: Jonathan Nieder \n---\nThat's the end of the series. This last one should probably have\nbeen called 7/6 or sent separately as an RFC, since it's more a\nmatter of taste. It was just something I ran into while trying out\nthe tests.\n\nHopefully the series wasn't too boring. Thanks for reading.\n\n t/test-lib.sh | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8a274fb..a174f66 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -731,12 +731,11 @@ test_expect_code () {\n \texit_code=$?\n \tif test $exit_code = $want_code\n \tthen\n-\t\techo >&2 \"test_expect_code: command exited with $exit_code: $*\"\n \t\treturn 0\n-\telse\n-\t\techo >&2 \"test_expect_code: command exited with $exit_code, we wanted $want_code $*\"\n-\t\treturn 1\n \tfi\n+\n+\techo >&2 \"test_expect_code: command exited with $exit_code, we wanted $want_code $*\"\n+\treturn 1\n }\n \n # test_cmp is a helper function to compare actual and expected output.\n-- \n1.7.5.1\n"},{"id":"168441","messageId":"BANLkTik0KS=JRjgKXxjuUhRx4TVv8d8cQg@mail.gmail.com","threadId":"27361","inReplyTo":"20110521185314.GA10530@elie","subject":"Re: [PATCH 0/7] Re: Problems with Git's \"perl\" userdiff driver","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-05-22T08:04:53Z","receivedAt":"2011-05-22T08:04:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, May 21, 2011 at 20:53, Jonathan Nieder wrote:\n\n> Jonathan Nieder (7):\n>  t4018 (diff funcname patterns): make .gitattributes state easier to\n>    track\n>  t4018 (diff funcname patterns): make configuration easier to track\n>  t4018 (diff funcname patterns): minor tweaks\n>  userdiff/perl: anchor \"sub\" and \"package\" patterns on the left\n>  userdiff/perl: match full line of POD headers\n>  userdiff/perl: catch subs with brace on second line\n>  tests: make test_expect_code quieter on success\n\nThis series gets my acked-by, it's a good improvement.\n\nI agree that we should have some sort of scoring mechanism so that the\nroot cause of these issues can be solved. But that's something for\nanother series.\n"},{"id":"168449","messageId":"20110522172932.GA21159@elie","threadId":"27361","inReplyTo":"20110521193826.GG10530@elie","subject":"[PATCH 8/7] userdiff/perl: tighten BEGIN/END block pattern to reject here-doc delimiters","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-22T17:29:32Z","receivedAt":"2011-05-22T17:29:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A naive method of treating BEGIN/END blocks with a brace on the second\nline as diff/grep funcname context involves also matching unrelated\nlines that consist of all-caps letters:\n\n\tsub foo {\n\t\tprint <<'EOF'\n\ttext goes here\n\t...\n\tEOF\n\t\t... rest of foo ...\n\t}\n\nThat's not so great, because it means that \"git diff\" and \"git grep\n--show-function\" would write \"=EOF\" or \"@@ EOF\" as context instead of\na more useful reminder like \"@@ sub foo {\".\n\nTo avoid this, tighten the pattern to only match the special block\nnames that perl accepts (namely BEGIN, END, INIT, CHECK, UNITCHECK,\nAUTOLOAD, and DESTROY). The list is taken from perl's toke.c.\n\nSuggested-by: Jakub Narebski \nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n\n> Accept\n>\n> \tsub foo\n> \t{\n> \t}\n>\n> as an alternative to a more common style that introduces perl\n> functions with a brace on the first line (and likewise for BEGIN/END\n> blocks).\n\nI just remembered that this pattern is overzealous since it will match\nhere-document terminators, as in:\n\n\tsub foo {\n\t\tprint <<'FINIS'\n\there-doc goes here\n\tFINIS\n\t\t... rest of foo, a change to which might be described in\n\t\ta diff ...\n\t}\n\nFix follows.\n\n t/t4018-diff-funcname.sh | 17 +++++++++++++++--\n userdiff.c | 2 +-\n 2 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex b2fd1a9..b68c56b 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -29,7 +29,7 @@ public class Beer\n }\n EOF\n sed 's/beer\\\\/beer,\\\\/' Beer-correct.java\n-cat >Beer.perl <<\\EOF\n+cat >Beer.perl <<\\EOT\n package Beer;\n \n use strict;\n@@ -56,6 +56,15 @@ sub finalround\n \tprint \"99 bottles of beer on the wall.\\n\");\n }\n \n+sub withheredocument {\n+\tprint <<\"EOF\"\n+decoy here-doc\n+EOF\n+\t# some lines of context\n+\t# to pad it out\n+\tprint \"hello\\n\";\n+}\n+\n __END__\n \n =head1 NAME\n@@ -76,7 +85,7 @@ Beer - subroutine to output fragment of a drinking song\n \tsong;\n \n =cut\n-EOF\n+EOT\n sed -e '\n \ts/hello/goodbye/\n \ts/beer\\\\/beer,\\\\/\n@@ -138,6 +147,10 @@ test_expect_success 'perl pattern accepts K&R style brace placement, too' '\n \ttest_expect_funcname \"sub finalround\\$\" perl\n '\n \n+test_expect_success 'but is not distracted by end of <