From 407e07f2a6f55e605fda9e90cb622887269f68b5 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:13 +0100 Subject: [PATCH 01/10] userdiff: support C++ ->* and .* operators in the word regexp The character sequences ->* and .* are valid C++ operators. Keep them together in --word-diff mode. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- userdiff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/userdiff.c b/userdiff.c index 10b61ec37d..434535bd63 100644 --- a/userdiff.c +++ b/userdiff.c @@ -133,7 +133,7 @@ PATTERNS("cpp", /* -- */ "[a-zA-Z_][a-zA-Z0-9_]*" "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?" - "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"), + "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->\\*?|\\.\\*"), PATTERNS("csharp", /* Keywords */ "!^[ \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n" From abf8f9860248d8c213600974742f18dadaa8fbb5 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:14 +0100 Subject: [PATCH 02/10] userdiff: support unsigned and long long suffixes of integer constants Do not split constants such as 123U, 456ll, 789UL at the first U or second L. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- userdiff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/userdiff.c b/userdiff.c index 434535bd63..8830417e3b 100644 --- a/userdiff.c +++ b/userdiff.c @@ -132,7 +132,7 @@ PATTERNS("cpp", "^((struct|class|enum)[^;]*)$", /* -- */ "[a-zA-Z_][a-zA-Z0-9_]*" - "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?" + "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*" "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->\\*?|\\.\\*"), PATTERNS("csharp", /* Keywords */ From bfa7d01413bd02b5b3675ef0e96f764064a13ce8 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:15 +0100 Subject: [PATCH 03/10] t4018: an infrastructure to test hunk headers Add an infrastructure that simplifies adding new tests of the hunk header regular expressions. To add new tests, a file with the syntax to test can be dropped in the directory t4018. The README file explains how a test file must contain; the README itself tests the default behavior. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 60 ++++++++++++++++++++++++++++++++++++---- t/t4018/README | 18 ++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 t/t4018/README diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 38a092a0da..b467d9eb57 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -100,7 +100,25 @@ test_expect_funcname () { grep "^@@.*@@ $1" diff } -for p in ada bibtex cpp csharp fortran html java matlab objc pascal perl php python ruby tex +diffpatterns=" + ada + bibtex + cpp + csharp + fortran + html + java + matlab + objc + pascal + perl + php + python + ruby + tex +" + +for p in $diffpatterns do test_expect_success "builtin $p pattern compiles" ' echo "*.java diff=$p" >.gitattributes && @@ -118,11 +136,6 @@ do ' done -test_expect_success 'default behaviour' ' - rm -f .gitattributes && - test_expect_funcname "public class Beer\$" -' - test_expect_success 'set up .gitattributes declaring drivers to test' ' cat >.gitattributes <<-\EOF *.java diff=java @@ -182,4 +195,39 @@ test_expect_success 'alternation in pattern' ' test_expect_funcname "public static void main(" ' +test_expect_success 'setup hunk header tests' ' + for i in $diffpatterns + do + echo "$i-* diff=$i" + done > .gitattributes && + + # add all test files to the index + ( + cd "$TEST_DIRECTORY"/t4018 && + git --git-dir="$TRASH_DIRECTORY/.git" add . + ) && + + # place modified files in the worktree + for i in $(git ls-files) + do + sed -e "s/ChangeMe/IWasChanged/" <"$TEST_DIRECTORY/t4018/$i" >"$i" || return 1 + done +' + +# check each individual file +for i in $(git ls-files) +do + if grep broken "$i" >/dev/null 2>&1 + then + result=failure + else + result=success + fi + test_expect_$result "hunk header: $i" " + test_when_finished 'cat actual' && # for debugging only + git diff -U1 $i >actual && + grep '@@ .* @@.*RIGHT' actual + " +done + test_done diff --git a/t/t4018/README b/t/t4018/README new file mode 100644 index 0000000000..283e01cca1 --- /dev/null +++ b/t/t4018/README @@ -0,0 +1,18 @@ +How to write RIGHT test cases +============================= + +Insert the word "ChangeMe" (exactly this form) at a distance of +at least two lines from the line that must appear in the hunk header. + +The text that must appear in the hunk header must contain the word +"right", but in all upper-case, like in the title above. + +To mark a test case that highlights a malfunction, insert the word +BROKEN in all lower-case somewhere in the file. + +This text is a bit twisted and out of order, but it is itself a +test case for the default hunk header pattern. Know what you are doing +if you change it. + +BTW, this tests that the head line goes to the hunk header, not the line +of equal signs. From 2d08413ba1ce0a1940e97a5a8af5faf40ab8d178 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:16 +0100 Subject: [PATCH 04/10] t4018: convert perl pattern tests to the new infrastructure There is one subtlety: The old test case 'perl pattern gets full line of POD header' does not have its own new test case, but the feature is tested nevertheless by placing the RIGHT tag at the end of the expected hunk header in t4018/perl-skip-sub-in-pod. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 88 ---------------------------- t/t4018/perl-skip-end-of-heredoc | 8 +++ t/t4018/perl-skip-forward-decl | 10 ++++ t/t4018/perl-skip-sub-in-pod | 18 ++++++ t/t4018/perl-sub-definition | 4 ++ t/t4018/perl-sub-definition-kr-brace | 4 ++ 6 files changed, 44 insertions(+), 88 deletions(-) create mode 100644 t/t4018/perl-skip-end-of-heredoc create mode 100644 t/t4018/perl-skip-forward-decl create mode 100644 t/t4018/perl-skip-sub-in-pod create mode 100644 t/t4018/perl-sub-definition create mode 100644 t/t4018/perl-sub-definition-kr-brace diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index b467d9eb57..c94a5f49d2 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -29,69 +29,6 @@ public class Beer } EOF sed 's/beer\\/beer,\\/' Beer-correct.java -cat >Beer.perl <<\EOT -package Beer; - -use strict; -use warnings; -use parent qw(Exporter); -our @EXPORT_OK = qw(round finalround); - -sub other; # forward declaration - -# hello - -sub round { - my ($n) = @_; - print "$n bottles of beer on the wall "; - print "$n bottles of beer\n"; - print "Take one down, pass it around, "; - $n = $n - 1; - 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"); -} - -sub withheredocument { - print <<"EOF" -decoy here-doc -EOF - # some lines of context - # to pad it out - print "hello\n"; -} - -__END__ - -=head1 NAME - -Beer - subroutine to output fragment of a drinking song - -=head1 SYNOPSIS - - use Beer qw(round finalround); - - sub song { - for (my $i = 99; $i > 0; $i--) { - round $i; - } - finalround; - } - - song; - -=cut -EOT -sed -e ' - s/hello/goodbye/ - s/beer\\/beer,\\/ - s/more\\/more,\\/ - s/song;/song();/ -' Beer-correct.perl test_expect_funcname () { lang=${2-java} @@ -139,7 +76,6 @@ done test_expect_success 'set up .gitattributes declaring drivers to test' ' cat >.gitattributes <<-\EOF *.java diff=java - *.perl diff=perl EOF ' @@ -147,30 +83,6 @@ test_expect_success 'preset java pattern' ' test_expect_funcname "public static void main(" ' -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 'but is not distracted by end of < 0; $i--) { + round $i; + } + finalround; + } + + ChangeMe; + +=cut diff --git a/t/t4018/perl-sub-definition b/t/t4018/perl-sub-definition new file mode 100644 index 0000000000..a507d1f645 --- /dev/null +++ b/t/t4018/perl-sub-definition @@ -0,0 +1,4 @@ +sub RIGHT { + my ($n) = @_; + print "ChangeMe"; +} diff --git a/t/t4018/perl-sub-definition-kr-brace b/t/t4018/perl-sub-definition-kr-brace new file mode 100644 index 0000000000..330b3df114 --- /dev/null +++ b/t/t4018/perl-sub-definition-kr-brace @@ -0,0 +1,4 @@ +sub RIGHT +{ + print "ChangeMe\n"; +} From dd4dc5c574c0849fef6855d9efdfae5dc9369f2d Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:17 +0100 Subject: [PATCH 05/10] t4018: convert java pattern test to the new infrastructure Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 4 ---- t/t4018/java-class-member-function | 8 ++++++++ 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 t/t4018/java-class-member-function diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index c94a5f49d2..008325f2a8 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -79,10 +79,6 @@ test_expect_success 'set up .gitattributes declaring drivers to test' ' EOF ' -test_expect_success 'preset java pattern' ' - test_expect_funcname "public static void main(" -' - test_expect_success 'custom pattern' ' test_config diff.java.funcname "!static !String diff --git a/t/t4018/java-class-member-function b/t/t4018/java-class-member-function new file mode 100644 index 0000000000..298bc7a71b --- /dev/null +++ b/t/t4018/java-class-member-function @@ -0,0 +1,8 @@ +public class Beer +{ + int special; + public static void main(String RIGHT[]) + { + System.out.print("ChangeMe"); + } +} From f1b75fbaf1380e00668f775f27ba8dd52b78a081 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:18 +0100 Subject: [PATCH 06/10] t4018: convert custom pattern test to the new infrastructure For the test case "matches to end of line", extend the pattern by a few wildcards so that the pattern captures the "RIGHT" token, which is needed for verification, without mentioning it in the pattern. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 40 +++++++++++--------------- t/t4018/custom1-pattern | 17 +++++++++++ t/t4018/custom2-match-to-end-of-line | 8 ++++++ t/t4018/custom3-alternation-in-pattern | 17 +++++++++++ 4 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 t/t4018/custom1-pattern create mode 100644 t/t4018/custom2-match-to-end-of-line create mode 100644 t/t4018/custom3-alternation-in-pattern diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 008325f2a8..5ac744f397 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -30,12 +30,19 @@ public class Beer EOF sed 's/beer\\/beer,\\/' Beer-correct.java -test_expect_funcname () { - lang=${2-java} - test_expect_code 1 git diff --no-index -U1 \ - "Beer.$lang" "Beer-correct.$lang" >diff && - grep "^@@.*@@ $1" diff -} +test_expect_success 'setup' ' + # a non-trivial custom pattern + git config diff.custom1.funcname "!static +!String +[^ ].*s.*" && + + # a custom pattern which matches to end of line + git config diff.custom2.funcname "......Beer\$" && + + # alternation in pattern + git config diff.custom3.funcname "Beer$" && + git config diff.custom3.xfuncname "^[ ]*((public|static).*)$" +' diffpatterns=" ada @@ -53,6 +60,9 @@ diffpatterns=" python ruby tex + custom1 + custom2 + custom3 " for p in $diffpatterns @@ -79,30 +89,12 @@ test_expect_success 'set up .gitattributes declaring drivers to test' ' EOF ' -test_expect_success 'custom pattern' ' - test_config diff.java.funcname "!static -!String -[^ ].*s.*" && - test_expect_funcname "int special;\$" -' - test_expect_success 'last regexp must not be negated' ' test_config diff.java.funcname "!static" && test_expect_code 128 git diff --no-index Beer.java Beer-correct.java 2>msg && grep ": Last expression must not be negated:" msg ' -test_expect_success 'pattern which matches to end of line' ' - test_config diff.java.funcname "Beer\$" && - test_expect_funcname "Beer\$" -' - -test_expect_success 'alternation in pattern' ' - test_config diff.java.funcname "Beer$" && - test_config diff.java.xfuncname "^[ ]*((public|static).*)$" && - test_expect_funcname "public static void main(" -' - test_expect_success 'setup hunk header tests' ' for i in $diffpatterns do diff --git a/t/t4018/custom1-pattern b/t/t4018/custom1-pattern new file mode 100644 index 0000000000..e8fd59f884 --- /dev/null +++ b/t/t4018/custom1-pattern @@ -0,0 +1,17 @@ +public class Beer +{ + int special, RIGHT; + public static void main(String args[]) + { + String s=" "; + for(int x = 99; x > 0; x--) + { + System.out.print(x + " bottles of beer on the wall " + + x + " bottles of beer\n" // ChangeMe + + "Take one down, pass it around, " + (x - 1) + + " bottles of beer on the wall.\n"); + } + System.out.print("Go to the store, buy some more,\n" + + "99 bottles of beer on the wall.\n"); + } +} diff --git a/t/t4018/custom2-match-to-end-of-line b/t/t4018/custom2-match-to-end-of-line new file mode 100644 index 0000000000..f88ac318b7 --- /dev/null +++ b/t/t4018/custom2-match-to-end-of-line @@ -0,0 +1,8 @@ +public class RIGHT_Beer +{ + int special; + public static void main(String args[]) + { + System.out.print("ChangeMe"); + } +} diff --git a/t/t4018/custom3-alternation-in-pattern b/t/t4018/custom3-alternation-in-pattern new file mode 100644 index 0000000000..5f3769c64f --- /dev/null +++ b/t/t4018/custom3-alternation-in-pattern @@ -0,0 +1,17 @@ +public class Beer +{ + int special; + public static void main(String RIGHT[]) + { + String s=" "; + for(int x = 99; x > 0; x--) + { + System.out.print(x + " bottles of beer on the wall " + + x + " bottles of beer\n" // ChangeMe + + "Take one down, pass it around, " + (x - 1) + + " bottles of beer on the wall.\n"); + } + System.out.print("Go to the store, buy some more,\n" + + "99 bottles of beer on the wall.\n"); + } +} From ad5070fb363b1e7ee7e1df83a17f4e8e1eb607a0 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:19 +0100 Subject: [PATCH 07/10] t4018: reduce test files for pattern compilation tests All test cases that need a file with specific text patterns have been converted to utilize texts in the t4018/ directory. The remaining tests in the test script deal only with the validity of the regular expressions. These tests do not depend on the contents of files that 'git diff' is invoked on. Remove the largish here-document and use only tiny files. While we are touching these tests, convert grep to test_i18ngrep as the texts checked for may undergo translation in the future. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018-diff-funcname.sh | 52 +++++++++++----------------------------- 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 5ac744f397..34591c23da 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -7,29 +7,6 @@ test_description='Test custom diff function name patterns' . ./test-lib.sh -LF=' -' -cat >Beer.java <<\EOF -public class Beer -{ - int special; - public static void main(String args[]) - { - String s=" "; - for(int x = 99; x > 0; x--) - { - System.out.print(x + " bottles of beer on the wall " - + x + " bottles of beer\n" - + "Take one down, pass it around, " + (x - 1) - + " bottles of beer on the wall.\n"); - } - System.out.print("Go to the store, buy some more,\n" - + "99 bottles of beer on the wall.\n"); - } -} -EOF -sed 's/beer\\/beer,\\/' Beer-correct.java - test_expect_success 'setup' ' # a non-trivial custom pattern git config diff.custom1.funcname "!static @@ -41,7 +18,11 @@ test_expect_success 'setup' ' # alternation in pattern git config diff.custom3.funcname "Beer$" && - git config diff.custom3.xfuncname "^[ ]*((public|static).*)$" + git config diff.custom3.xfuncname "^[ ]*((public|static).*)$" && + + # for regexp compilation tests + echo A >A.java && + echo B >B.java ' diffpatterns=" @@ -70,29 +51,24 @@ do test_expect_success "builtin $p pattern compiles" ' echo "*.java diff=$p" >.gitattributes && test_expect_code 1 git diff --no-index \ - Beer.java Beer-correct.java 2>msg && - ! grep fatal msg && - ! grep error msg + A.java B.java 2>msg && + ! test_i18ngrep fatal msg && + ! test_i18ngrep error msg ' test_expect_success "builtin $p wordRegex pattern compiles" ' echo "*.java diff=$p" >.gitattributes && test_expect_code 1 git diff --no-index --word-diff \ - Beer.java Beer-correct.java 2>msg && - ! grep fatal msg && - ! grep error msg + A.java B.java 2>msg && + ! test_i18ngrep fatal msg && + ! test_i18ngrep error msg ' done -test_expect_success 'set up .gitattributes declaring drivers to test' ' - cat >.gitattributes <<-\EOF - *.java diff=java - EOF -' - test_expect_success 'last regexp must not be negated' ' + echo "*.java diff=java" >.gitattributes && test_config diff.java.funcname "!static" && - test_expect_code 128 git diff --no-index Beer.java Beer-correct.java 2>msg && - grep ": Last expression must not be negated:" msg + test_expect_code 128 git diff --no-index A.java B.java 2>msg && + test_i18ngrep ": Last expression must not be negated:" msg ' test_expect_success 'setup hunk header tests' ' From 02907a08ccfbdf0a9a48259fd7e9a234f3c123b3 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:20 +0100 Subject: [PATCH 08/10] t4018: test cases for the built-in cpp pattern A later patch changes the built-in cpp pattern. These test cases demonstrate aspects of the pattern that we do not want to change. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018/cpp-c++-function | 4 ++++ t/t4018/cpp-class-definition | 4 ++++ t/t4018/cpp-class-definition-derived | 5 +++++ t/t4018/cpp-function-returning-pointer | 4 ++++ t/t4018/cpp-skip-access-specifiers | 8 ++++++++ t/t4018/cpp-skip-comment-block | 9 +++++++++ t/t4018/cpp-skip-labels | 8 ++++++++ t/t4018/cpp-struct-definition | 9 +++++++++ t/t4018/cpp-void-c-function | 4 ++++ 9 files changed, 55 insertions(+) create mode 100644 t/t4018/cpp-c++-function create mode 100644 t/t4018/cpp-class-definition create mode 100644 t/t4018/cpp-class-definition-derived create mode 100644 t/t4018/cpp-function-returning-pointer create mode 100644 t/t4018/cpp-skip-access-specifiers create mode 100644 t/t4018/cpp-skip-comment-block create mode 100644 t/t4018/cpp-skip-labels create mode 100644 t/t4018/cpp-struct-definition create mode 100644 t/t4018/cpp-void-c-function diff --git a/t/t4018/cpp-c++-function b/t/t4018/cpp-c++-function new file mode 100644 index 0000000000..9ee6bbef55 --- /dev/null +++ b/t/t4018/cpp-c++-function @@ -0,0 +1,4 @@ +Item RIGHT::DoSomething( Args with_spaces ) +{ + ChangeMe; +} diff --git a/t/t4018/cpp-class-definition b/t/t4018/cpp-class-definition new file mode 100644 index 0000000000..11b61da3b7 --- /dev/null +++ b/t/t4018/cpp-class-definition @@ -0,0 +1,4 @@ +class RIGHT +{ + int ChangeMe; +}; diff --git a/t/t4018/cpp-class-definition-derived b/t/t4018/cpp-class-definition-derived new file mode 100644 index 0000000000..3b98cd09ab --- /dev/null +++ b/t/t4018/cpp-class-definition-derived @@ -0,0 +1,5 @@ +class RIGHT : + public Baseclass +{ + int ChangeMe; +}; diff --git a/t/t4018/cpp-function-returning-pointer b/t/t4018/cpp-function-returning-pointer new file mode 100644 index 0000000000..ef15657ea8 --- /dev/null +++ b/t/t4018/cpp-function-returning-pointer @@ -0,0 +1,4 @@ +const char *get_it_RIGHT(char *ptr) +{ + ChangeMe; +} diff --git a/t/t4018/cpp-skip-access-specifiers b/t/t4018/cpp-skip-access-specifiers new file mode 100644 index 0000000000..4d4a9dbb9d --- /dev/null +++ b/t/t4018/cpp-skip-access-specifiers @@ -0,0 +1,8 @@ +class RIGHT : public Baseclass +{ +public: +protected: +private: + void DoSomething(); + int ChangeMe; +}; diff --git a/t/t4018/cpp-skip-comment-block b/t/t4018/cpp-skip-comment-block new file mode 100644 index 0000000000..3800b9967a --- /dev/null +++ b/t/t4018/cpp-skip-comment-block @@ -0,0 +1,9 @@ +struct item RIGHT(int i) +// Do not +// pick up +/* these +** comments. +*/ +{ + ChangeMe; +} diff --git a/t/t4018/cpp-skip-labels b/t/t4018/cpp-skip-labels new file mode 100644 index 0000000000..b9c10aba22 --- /dev/null +++ b/t/t4018/cpp-skip-labels @@ -0,0 +1,8 @@ +void RIGHT (void) +{ +repeat: // C++ comment +next: /* C comment */ + do_something(); + + ChangeMe; +} diff --git a/t/t4018/cpp-struct-definition b/t/t4018/cpp-struct-definition new file mode 100644 index 0000000000..521c59fd15 --- /dev/null +++ b/t/t4018/cpp-struct-definition @@ -0,0 +1,9 @@ +struct RIGHT { + unsigned + /* this bit field looks like a label and should not be picked up */ + decoy_bitfield: 2, + more : 1; + int filler; + + int ChangeMe; +}; diff --git a/t/t4018/cpp-void-c-function b/t/t4018/cpp-void-c-function new file mode 100644 index 0000000000..153081e872 --- /dev/null +++ b/t/t4018/cpp-void-c-function @@ -0,0 +1,4 @@ +void RIGHT (void) +{ + ChangeMe; +} From 9cc444f0570b196f1c51664ce2de1d8e1dee6046 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:21 +0100 Subject: [PATCH 09/10] t4018: test cases showing that the cpp pattern misses many anchor points Most of the tests show C++ code, but there is also a union definition and a GNU style function definition that are not recognized. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018/cpp-class-constructor | 5 +++++ t/t4018/cpp-class-constructor-mem-init | 6 ++++++ t/t4018/cpp-class-destructor | 5 +++++ t/t4018/cpp-function-returning-global-type | 5 +++++ t/t4018/cpp-function-returning-nested | 6 ++++++ t/t4018/cpp-function-returning-reference | 5 +++++ t/t4018/cpp-gnu-style-function | 6 ++++++ t/t4018/cpp-namespace-definition | 5 +++++ t/t4018/cpp-operator-definition | 5 +++++ t/t4018/cpp-struct-single-line | 8 ++++++++ t/t4018/cpp-template-function-definition | 5 +++++ t/t4018/cpp-union-definition | 5 +++++ 12 files changed, 66 insertions(+) create mode 100644 t/t4018/cpp-class-constructor create mode 100644 t/t4018/cpp-class-constructor-mem-init create mode 100644 t/t4018/cpp-class-destructor create mode 100644 t/t4018/cpp-function-returning-global-type create mode 100644 t/t4018/cpp-function-returning-nested create mode 100644 t/t4018/cpp-function-returning-reference create mode 100644 t/t4018/cpp-gnu-style-function create mode 100644 t/t4018/cpp-namespace-definition create mode 100644 t/t4018/cpp-operator-definition create mode 100644 t/t4018/cpp-struct-single-line create mode 100644 t/t4018/cpp-template-function-definition create mode 100644 t/t4018/cpp-union-definition diff --git a/t/t4018/cpp-class-constructor b/t/t4018/cpp-class-constructor new file mode 100644 index 0000000000..4c4925c237 --- /dev/null +++ b/t/t4018/cpp-class-constructor @@ -0,0 +1,5 @@ +Item::Item(int RIGHT) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-class-constructor-mem-init b/t/t4018/cpp-class-constructor-mem-init new file mode 100644 index 0000000000..eec1d7cbf3 --- /dev/null +++ b/t/t4018/cpp-class-constructor-mem-init @@ -0,0 +1,6 @@ +Item::Item(int RIGHT) : + member(0) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-class-destructor b/t/t4018/cpp-class-destructor new file mode 100644 index 0000000000..03aa51ca5b --- /dev/null +++ b/t/t4018/cpp-class-destructor @@ -0,0 +1,5 @@ +RIGHT::~RIGHT() +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-function-returning-global-type b/t/t4018/cpp-function-returning-global-type new file mode 100644 index 0000000000..bff3e5f21c --- /dev/null +++ b/t/t4018/cpp-function-returning-global-type @@ -0,0 +1,5 @@ +::Item get::it::RIGHT() +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-function-returning-nested b/t/t4018/cpp-function-returning-nested new file mode 100644 index 0000000000..41700f2c09 --- /dev/null +++ b/t/t4018/cpp-function-returning-nested @@ -0,0 +1,6 @@ +get::Item get::it::RIGHT() +{ + ChangeMe; + broken; +} + diff --git a/t/t4018/cpp-function-returning-reference b/t/t4018/cpp-function-returning-reference new file mode 100644 index 0000000000..29e2bd4632 --- /dev/null +++ b/t/t4018/cpp-function-returning-reference @@ -0,0 +1,5 @@ +string& get::it::RIGHT(char *ptr) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-gnu-style-function b/t/t4018/cpp-gnu-style-function new file mode 100644 index 0000000000..d65fc7489c --- /dev/null +++ b/t/t4018/cpp-gnu-style-function @@ -0,0 +1,6 @@ +const char * +RIGHT(int arg) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-namespace-definition b/t/t4018/cpp-namespace-definition new file mode 100644 index 0000000000..6b88dd9c3b --- /dev/null +++ b/t/t4018/cpp-namespace-definition @@ -0,0 +1,5 @@ +namespace RIGHT +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-operator-definition b/t/t4018/cpp-operator-definition new file mode 100644 index 0000000000..f2bd1678f9 --- /dev/null +++ b/t/t4018/cpp-operator-definition @@ -0,0 +1,5 @@ +Value operator+(Value LEFT, Value RIGHT) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-struct-single-line b/t/t4018/cpp-struct-single-line new file mode 100644 index 0000000000..ad6fa8bbe9 --- /dev/null +++ b/t/t4018/cpp-struct-single-line @@ -0,0 +1,8 @@ +void wrong() +{ +} + +struct RIGHT_iterator_tag {}; + +int ChangeMe; +// broken diff --git a/t/t4018/cpp-template-function-definition b/t/t4018/cpp-template-function-definition new file mode 100644 index 0000000000..a410298b0b --- /dev/null +++ b/t/t4018/cpp-template-function-definition @@ -0,0 +1,5 @@ +template int RIGHT(T arg) +{ + ChangeMe; + broken; +} diff --git a/t/t4018/cpp-union-definition b/t/t4018/cpp-union-definition new file mode 100644 index 0000000000..133b662258 --- /dev/null +++ b/t/t4018/cpp-union-definition @@ -0,0 +1,5 @@ +union RIGHT { + double v; + int ChangeMe; + broken; +}; From 8a2e8da367f7175465118510b474ad365161d6b1 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Fri, 21 Mar 2014 22:07:22 +0100 Subject: [PATCH 10/10] userdiff: have 'cpp' hunk header pattern catch more C++ anchor points The hunk header pattern 'cpp' is intended for C and C++ source code, but it is actually not particularly useful for the latter, and even misses some use-cases for the former. The parts of the pattern have the following flaws: - The first part matches an identifier followed immediately by a colon and arbitrary text and is intended to reject goto labels and C++ access specifiers (public, private, protected). But this pattern also rejects C++ constructs, which look like this: MyClass::MyClass() MyClass::~MyClass() MyClass::Item MyClass::Find(... - The second part matches an identifier followed by a list of qualified names (i.e. identifiers separated by the C++ scope operator '::') separated by space or '*' followed by an opening parenthesis (with space between the tokens). It matches function declarations like struct item* get_head(... int Outer::Inner::Func(... Since the pattern requires at least two identifiers, GNU-style function definitions are ignored: void func(... Moreover, since the pattern does not allow punctuation other than '*', the following C++ constructs are not recognized: . template definitions: template int func(T arg) . functions returning references: const string& get_message() . functions returning templated types: vector foo() . operator definitions: Value operator+(Value l, Value r) - The third part of the pattern finally matches compound definitions. But it forgets about unions and namespaces, and also skips single-line definitions struct random_iterator_tag {}; because no semicolon can occur on the line. Change the first pattern to require a colon at the end of the line (except for trailing space and comments), so that it does not reject constructor or destructor definitions. Notice that all interesting anchor points begin with an identifier or keyword. But since there is a large variety of syntactical constructs after the first "word", the simplest is to require only this word and accept everything else. Therefore, this boils down to a line that begins with a letter or underscore (optionally preceded by the C++ scope operator '::' to accept functions returning a type anchored at the global namespace). Replace the second and third part by a single pattern that picks such a line. This has the following desirable consequence: - All constructs mentioned above are recognized. and the following likely desirable consequences: - Definitions of global variables and typedefs are recognized: int num_entries = 0; extern const char* help_text; typedef basic_string wstring; - Commonly used marco-ized boilerplate code is recognized: BEGIN_MESSAGE_MAP(CCanvas,CWnd) Q_DECLARE_METATYPE(MyStruct) PATTERNS("tex",...) (The last one is from this very patch.) but also the following possibly undesirable consequence: - When a label is not on a line by itself (except for a comment) it is no longer rejected, but can appear as a hunk header if it occurs at the beginning of a line: next:; IMO, the benefits of the change outweigh the (possible) regressions by a large margin. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t4018/cpp-class-constructor | 1 - t/t4018/cpp-class-constructor-mem-init | 1 - t/t4018/cpp-class-destructor | 1 - t/t4018/cpp-function-returning-global-type | 1 - t/t4018/cpp-function-returning-nested | 1 - t/t4018/cpp-function-returning-reference | 1 - t/t4018/cpp-gnu-style-function | 1 - t/t4018/cpp-namespace-definition | 1 - t/t4018/cpp-operator-definition | 1 - t/t4018/cpp-struct-single-line | 1 - t/t4018/cpp-template-function-definition | 1 - t/t4018/cpp-union-definition | 1 - userdiff.c | 8 +++----- 13 files changed, 3 insertions(+), 17 deletions(-) diff --git a/t/t4018/cpp-class-constructor b/t/t4018/cpp-class-constructor index 4c4925c237..ec4f115c25 100644 --- a/t/t4018/cpp-class-constructor +++ b/t/t4018/cpp-class-constructor @@ -1,5 +1,4 @@ Item::Item(int RIGHT) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-class-constructor-mem-init b/t/t4018/cpp-class-constructor-mem-init index eec1d7cbf3..49a69f37e1 100644 --- a/t/t4018/cpp-class-constructor-mem-init +++ b/t/t4018/cpp-class-constructor-mem-init @@ -2,5 +2,4 @@ Item::Item(int RIGHT) : member(0) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-class-destructor b/t/t4018/cpp-class-destructor index 03aa51ca5b..5487665096 100644 --- a/t/t4018/cpp-class-destructor +++ b/t/t4018/cpp-class-destructor @@ -1,5 +1,4 @@ RIGHT::~RIGHT() { ChangeMe; - broken; } diff --git a/t/t4018/cpp-function-returning-global-type b/t/t4018/cpp-function-returning-global-type index bff3e5f21c..1084d5990e 100644 --- a/t/t4018/cpp-function-returning-global-type +++ b/t/t4018/cpp-function-returning-global-type @@ -1,5 +1,4 @@ ::Item get::it::RIGHT() { ChangeMe; - broken; } diff --git a/t/t4018/cpp-function-returning-nested b/t/t4018/cpp-function-returning-nested index 41700f2c09..d9750aa61a 100644 --- a/t/t4018/cpp-function-returning-nested +++ b/t/t4018/cpp-function-returning-nested @@ -1,6 +1,5 @@ get::Item get::it::RIGHT() { ChangeMe; - broken; } diff --git a/t/t4018/cpp-function-returning-reference b/t/t4018/cpp-function-returning-reference index 29e2bd4632..01b051df70 100644 --- a/t/t4018/cpp-function-returning-reference +++ b/t/t4018/cpp-function-returning-reference @@ -1,5 +1,4 @@ string& get::it::RIGHT(char *ptr) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-gnu-style-function b/t/t4018/cpp-gnu-style-function index d65fc7489c..08c7c7565a 100644 --- a/t/t4018/cpp-gnu-style-function +++ b/t/t4018/cpp-gnu-style-function @@ -2,5 +2,4 @@ const char * RIGHT(int arg) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-namespace-definition b/t/t4018/cpp-namespace-definition index 6b88dd9c3b..6749980241 100644 --- a/t/t4018/cpp-namespace-definition +++ b/t/t4018/cpp-namespace-definition @@ -1,5 +1,4 @@ namespace RIGHT { ChangeMe; - broken; } diff --git a/t/t4018/cpp-operator-definition b/t/t4018/cpp-operator-definition index f2bd1678f9..1acd827159 100644 --- a/t/t4018/cpp-operator-definition +++ b/t/t4018/cpp-operator-definition @@ -1,5 +1,4 @@ Value operator+(Value LEFT, Value RIGHT) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-struct-single-line b/t/t4018/cpp-struct-single-line index ad6fa8bbe9..a0de5fb800 100644 --- a/t/t4018/cpp-struct-single-line +++ b/t/t4018/cpp-struct-single-line @@ -5,4 +5,3 @@ void wrong() struct RIGHT_iterator_tag {}; int ChangeMe; -// broken diff --git a/t/t4018/cpp-template-function-definition b/t/t4018/cpp-template-function-definition index a410298b0b..0cdf5ba5bd 100644 --- a/t/t4018/cpp-template-function-definition +++ b/t/t4018/cpp-template-function-definition @@ -1,5 +1,4 @@ template int RIGHT(T arg) { ChangeMe; - broken; } diff --git a/t/t4018/cpp-union-definition b/t/t4018/cpp-union-definition index 133b662258..7ec94df697 100644 --- a/t/t4018/cpp-union-definition +++ b/t/t4018/cpp-union-definition @@ -1,5 +1,4 @@ union RIGHT { double v; int ChangeMe; - broken; }; diff --git a/userdiff.c b/userdiff.c index 8830417e3b..fad52d6392 100644 --- a/userdiff.c +++ b/userdiff.c @@ -125,11 +125,9 @@ PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$", "\\\\[a-zA-Z@]+|\\\\.|[a-zA-Z0-9\x80-\xff]+"), PATTERNS("cpp", /* Jump targets or access declarations */ - "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n" - /* C/++ functions/methods at top level */ - "^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n" - /* compound type at top level */ - "^((struct|class|enum)[^;]*)$", + "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\n" + /* functions/methods, variables, and compounds at top level */ + "^((::[[:space:]]*)?[A-Za-z_].*)$", /* -- */ "[a-zA-Z_][a-zA-Z0-9_]*" "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*"