From f12c66b9bb851aa7350d40370e6adf78535c5930 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:29:01 -0500 Subject: userdiff/perl: anchor "sub" and "package" patterns on the left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The userdiff funcname mechanism has no concept of nested scopes --- instead, "git diff" and "git grep --show-function" simply label the diff header with the most recent matching line. Unfortunately that means text following a subroutine in a POD section: =head1 DESCRIPTION You might use this facility like so: sub example { foo; } Now, having said that, let's say more about the facility. Blah blah blah ... etc etc. gets the subroutine name instead of the POD header in its diff/grep funcname header, making it harder to get oriented when reading a diff without enough context. The fix is simple: anchor the funcname syntax to the left margin so nested subroutines and packages like this won't get picked up. (The builtin C++ funcname pattern already does the same thing.) This means the userdiff driver will misparse the idiom { my $static; sub foo { ... use $static ... } } but I think that's worth it; we can revisit this later if the userdiff mechanism learns to keep track of the beginning and end of nested scopes. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- userdiff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'userdiff.c') diff --git a/userdiff.c b/userdiff.c index 1ff47977d5..2cca0af8e2 100644 --- a/userdiff.c +++ b/userdiff.c @@ -60,8 +60,8 @@ PATTERNS("pascal", "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+" "|<>|<=|>=|:=|\\.\\."), PATTERNS("perl", - "^[ \t]*package .*;\n" - "^[ \t]*sub .* \\{\n" + "^package .*;\n" + "^sub .* \\{\n" "^[A-Z]+ \\{\n" /* BEGIN, END, ... */ "^=head[0-9] ", /* POD */ /* -- */ -- cgit v1.3 From 12f0967a8a1e3c11c678de181f77d1c7883b37cf Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:35:51 -0500 Subject: userdiff/perl: match full line of POD headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builtin perl userdiff driver is not greedy enough about catching POD header lines. Capture the whole line, so instead of just declaring that we are in some "@@ =head1" section, diff/grep output can explain that the enclosing section is about "@@ =head1 OPTIONS". Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 4 ++++ userdiff.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'userdiff.c') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index f071a8fdd1..8a5714912d 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -125,6 +125,10 @@ test_expect_success 'perl pattern is not distracted by sub within POD' ' test_expect_funcname "=head" perl ' +test_expect_success 'perl pattern gets full line of POD header' ' + test_expect_funcname "=head1 SYNOPSIS\$" perl +' + test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String diff --git a/userdiff.c b/userdiff.c index 2cca0af8e2..32ead9654a 100644 --- a/userdiff.c +++ b/userdiff.c @@ -63,7 +63,7 @@ PATTERNS("perl", "^package .*;\n" "^sub .* \\{\n" "^[A-Z]+ \\{\n" /* BEGIN, END, ... */ - "^=head[0-9] ", /* POD */ + "^=head[0-9] .*", /* POD */ /* -- */ "[[:alpha:]_'][[:alnum:]_']*" "|0[xb]?[0-9a-fA-F_]*" -- cgit v1.3 From ea2ca4497bdb716977a3e2526780635cb6bac513 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 21 May 2011 14:38:26 -0500 Subject: userdiff/perl: catch sub with brace on second line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept sub foo { } as an alternative to a more common style that introduces perl functions with a brace on the first line (and likewise for BEGIN/END blocks). The new regex is a little hairy to avoid matching # forward declaration sub foo; while continuing to match "sub foo($;@) {" and sub foo { # This routine is interesting; # in fact, the lines below explain how... While at it, pay attention to Perl 5.14's "package foo {" syntax as an alternative to the traditional "package foo;". Requested-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++-- userdiff.c | 20 +++++++++++++++++--- 2 files changed, 40 insertions(+), 5 deletions(-) (limited to 'userdiff.c') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 8a5714912d..b2fd1a99da 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -35,7 +35,11 @@ package Beer; use strict; use warnings; use parent qw(Exporter); -our @EXPORT_OK = qw(round); +our @EXPORT_OK = qw(round finalround); + +sub other; # forward declaration + +# hello sub round { my ($n) = @_; @@ -46,6 +50,12 @@ sub round { print "$n bottles of beer on the wall.\n"; } +sub finalround +{ + print "Go to the store, buy some more\n"; + print "99 bottles of beer on the wall.\n"); +} + __END__ =head1 NAME @@ -54,12 +64,13 @@ Beer - subroutine to output fragment of a drinking song =head1 SYNOPSIS - use Beer qw(round); + use Beer qw(round finalround); sub song { for (my $i = 99; $i > 0; $i--) { round $i; } + finalround; } song; @@ -67,7 +78,9 @@ Beer - subroutine to output fragment of a drinking song =cut EOF sed -e ' + s/hello/goodbye/ s/beer\\/beer,\\/ + s/more\\/more,\\/ s/song;/song();/ ' Beer-correct.perl @@ -121,6 +134,10 @@ test_expect_success 'preset perl pattern' ' test_expect_funcname "sub round {\$" perl ' +test_expect_success 'perl pattern accepts K&R style brace placement, too' ' + test_expect_funcname "sub finalround\$" perl +' + test_expect_success 'perl pattern is not distracted by sub within POD' ' test_expect_funcname "=head" perl ' @@ -129,6 +146,10 @@ test_expect_success 'perl pattern gets full line of POD header' ' test_expect_funcname "=head1 SYNOPSIS\$" perl ' +test_expect_success 'perl pattern is not distracted by forward declaration' ' + test_expect_funcname "package Beer;\$" perl +' + test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String diff --git a/userdiff.c b/userdiff.c index 32ead9654a..42b86ac63d 100644 --- a/userdiff.c +++ b/userdiff.c @@ -60,9 +60,23 @@ PATTERNS("pascal", "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+" "|<>|<=|>=|:=|\\.\\."), PATTERNS("perl", - "^package .*;\n" - "^sub .* \\{\n" - "^[A-Z]+ \\{\n" /* BEGIN, END, ... */ + "^package .*\n" + "^sub [[:alnum:]_':]+[ \t]*" + "(\\([^)]*\\)[ \t]*)?" /* prototype */ + /* + * Attributes. A regex can't count nested parentheses, + * so just slurp up whatever we see, taking care not + * to accept lines like "sub foo; # defined elsewhere". + * + * An attribute could contain a semicolon, but at that + * point it seems reasonable enough to give up. + */ + "(:[^;#]*)?" + "(\\{[ \t]*)?" /* brace can come here or on the next line */ + "(#.*)?$\n" /* comment */ + "^[A-Z]+[ \t]*" /* BEGIN, END, ... */ + "(\\{[ \t]*)?" /* brace can come here or on the next line */ + "(#.*)?$\n" "^=head[0-9] .*", /* POD */ /* -- */ "[[:alpha:]_'][[:alnum:]_']*" -- cgit v1.3 From f143d9c695cd4c3e86069c536fa0dff04fc93e93 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 22 May 2011 12:29:32 -0500 Subject: userdiff/perl: tighten BEGIN/END block pattern to reject here-doc delimiters A naive method of treating BEGIN/END blocks with a brace on the second line as diff/grep funcname context involves also matching unrelated lines that consist of all-caps letters: sub foo { print <<'EOF' text goes here ... EOF ... rest of foo ... } That's not so great, because it means that "git diff" and "git grep --show-function" would write "=EOF" or "@@ EOF" as context instead of a more useful reminder like "@@ sub foo {". To avoid this, tighten the pattern to only match the special block names that perl accepts (namely BEGIN, END, INIT, CHECK, UNITCHECK, AUTOLOAD, and DESTROY). The list is taken from perl's toke.c. Suggested-by: Jakub Narebski Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 17 +++++++++++++++-- userdiff.c | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) (limited to 'userdiff.c') diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index b2fd1a99da..b68c56b68c 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -29,7 +29,7 @@ public class Beer } EOF sed 's/beer\\/beer,\\/' Beer-correct.java -cat >Beer.perl <<\EOF +cat >Beer.perl <<\EOT package Beer; use strict; @@ -56,6 +56,15 @@ sub finalround print "99 bottles of beer on the wall.\n"); } +sub withheredocument { + print <<"EOF" +decoy here-doc +EOF + # some lines of context + # to pad it out + print "hello\n"; +} + __END__ =head1 NAME @@ -76,7 +85,7 @@ Beer - subroutine to output fragment of a drinking song song; =cut -EOF +EOT sed -e ' s/hello/goodbye/ s/beer\\/beer,\\/ @@ -138,6 +147,10 @@ test_expect_success 'perl pattern accepts K&R style brace placement, too' ' test_expect_funcname "sub finalround\$" perl ' +test_expect_success 'but is not distracted by end of <