git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jakub Narebski <jnareb@gmail.com>,
	Jeff King <peff@peff.net>
Subject: [PATCH 8/7] userdiff/perl: tighten BEGIN/END block pattern to reject here-doc delimiters
Date: Sun, 22 May 2011 12:29:32 -0500	[thread overview]
Message-ID: <20110522172932.GA21159@elie> (raw)
In-Reply-To: <20110521193826.GG10530@elie>

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 <jnareb@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:

> 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).

I just remembered that this pattern is overzealous since it will match
here-document terminators, as in:

	sub foo {
		print <<'FINIS'
	here-doc goes here
	FINIS
		... rest of foo, a change to which might be described in
		a diff ...
	}

Fix follows.

 t/t4018-diff-funcname.sh |   17 +++++++++++++++--
 userdiff.c               |    2 +-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index b2fd1a9..b68c56b 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.java >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 <<here document' '
+	test_expect_funcname "sub withheredocument {\$" perl
+'
+
 test_expect_success 'perl pattern is not distracted by sub within POD' '
 	test_expect_funcname "=head" perl
 '
diff --git a/userdiff.c b/userdiff.c
index 42b86ac..e55310c 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -74,7 +74,7 @@ PATTERNS("perl",
 		"(:[^;#]*)?"
 		"(\\{[ \t]*)?" /* brace can come here or on the next line */
 		"(#.*)?$\n" /* comment */
-	 "^[A-Z]+[ \t]*"	/* BEGIN, END, ... */
+	 "^(BEGIN|END|INIT|CHECK|UNITCHECK|AUTOLOAD|DESTROY)[ \t]*"
 		"(\\{[ \t]*)?" /* brace can come here or on the next line */
 		"(#.*)?$\n"
 	 "^=head[0-9] .*",	/* POD */
-- 
1.7.5.1

  reply	other threads:[~2011-05-22 17:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-15 18:14 Problems with Git's "perl" userdiff driver Ævar Arnfjörð Bjarmason
2011-05-15 20:02 ` Junio C Hamano
2011-05-15 20:13   ` Ævar Arnfjörð Bjarmason
2011-05-21 18:53 ` [PATCH 0/7] " Jonathan Nieder
2011-05-21 19:11   ` [PATCH 1/7] t4018 (funcname patterns): make .gitattributes state easier to track Jonathan Nieder
2011-05-21 19:22   ` [PATCH 2/7] t4018 (funcname patterns): make configuration " Jonathan Nieder
2011-05-21 19:25   ` [PATCH 3/7] t4018 (funcname patterns): minor cleanups Jonathan Nieder
2011-05-21 19:29   ` [PATCH 4/7] userdiff/perl: anchor "sub" and "package" patterns on the left Jonathan Nieder
2011-05-21 19:35   ` [PATCH 5/7] userdiff/perl: match full line of POD headers Jonathan Nieder
2011-05-21 19:38   ` [PATCH 6/7] userdiff/perl: catch sub with brace on second line Jonathan Nieder
2011-05-22 17:29     ` Jonathan Nieder [this message]
2011-05-21 19:40   ` [PATCH 7/7] tests: make test_expect_code quieter on success Jonathan Nieder
2011-05-22  8:04   ` [PATCH 0/7] Re: Problems with Git's "perl" userdiff driver Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110522172932.GA21159@elie \
    --to=jrnieder@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).