threads / patch / 19890

patchAdd matchings for csharp

Subject: [PATCH] Add matchings for csharp

## tl;dr

3 messages between Jun 21, 2009 and Jun 22, 2009. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Adam Petaccia· Jun 21, 2009, 23:52 UTC · lore

I'm not sure if this is the correct place to send patches, please direct 
me if I've addressed the wrong list.
---
 t/t4018-diff-funcname.sh |    2 +-
 userdiff.c               |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

Show changes to 2 files +11 −1

t/t4018-diff-funcname.sh, userdiff.c

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 5b10e97..704dae8 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -32,7 +32,7 @@ EOF
 
 sed 's/beer\\/beer,\\/' < Beer.java > Beer-correct.java
 
-builtin_patterns="bibtex cpp html java objc pascal php python ruby tex"
+builtin_patterns="bibtex csharp cpp html java objc pascal php python ruby tex"
 for p in $builtin_patterns
 do
 	test_expect_success "builtin $p pattern compiles" '
diff --git a/userdiff.c b/userdiff.c
index 57529ae..d4151b2 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -79,6 +79,16 @@ PATTERNS("cpp",
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
 	 "|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("csharp",
+	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
+	 "^([ \t]*(static|public|internal|private|protected|new|unsafe|readonly|volatile)[ \t]+(class|enum|interface|struct).*)$\n"
+	 "^([ \t]*(namespace)[ \t]+.*)$"
+	 ,
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
+	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
+	 "|[^[:space:]]|[\x80-\xff]+"),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS

-- 
1.6.0.4

Johannes Sixt· Jun 22, 2009, 06:24 UTC · re: Adam Petaccia · lore

Re: [PATCH] Add matchings for csharp

Thank you for you contribution.

Adam Petaccia schrieb:

> I'm not sure if this is the correct place to send patches, please direct 
> me if I've addressed the wrong list.

This part of a patch submission should contain a description *why* this patch is an improvement (and must include a Signed-off-by statement; see Documentation/SubmittingPatches). Perhaps you are aware of msysgit issue 281[1], that you are addressing here, and you could mention it. Considering the complaint in that issue (that hunk headers always only mention the containing namespace), an example in the commit message would be useful.

To answer you question: This is the right place.

[1] http://code.google.com/p/msysgit/issues/detail?id=281

Show 8 quoted lines

> @@ -79,6 +79,16 @@ PATTERNS("cpp",
>  	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>  	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
>  	 "|[^[:space:]]|[\x80-\xff]+"),
> +PATTERNS("csharp",
> +	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
> +	 "^([ \t]*(static|public|internal|private|protected|new|unsafe|readonly|volatile)[ \t]+(class|enum|interface|struct).*)$\n"
> +	 "^([ \t]*(namespace)[ \t]+.*)$"

I think the placement of parentheses could be improved in these two expressions (but I don't know for sure because I don't know the code well enough). Looking at the other examples in userdiff.c, I infer that the *first* opening parentheses defines which part is copied to the hunk header. Since you placed everything into parentheses (unnecessarily, I think), the complete line would be copied to the hunk header, including leading whitespace.

Disclaimer: I don't have C# source code to test this.

-- Hannes

Paolo Bonzini· Jun 22, 2009, 08:12 UTC · re: Johannes Sixt · lore

Re: [PATCH] Add matchings for csharp

Show 14 quoted lines

>> @@ -79,6 +79,16 @@ PATTERNS("cpp",
>>  	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
>>  	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
>>  	 "|[^[:space:]]|[\x80-\xff]+"),
>> +PATTERNS("csharp",
>> +	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
>> +	 "^([ \t]*(static|public|internal|private|protected|new|unsafe|readonly|volatile)[ \t]+(class|enum|interface|struct).*)$\n"
>> +	 "^([ \t]*(namespace)[ \t]+.*)$"
> 
> I think the placement of parentheses could be improved in these two
> expressions (but I don't know for sure because I don't know the code well
> enough). Looking at the other examples in userdiff.c, I infer that the
> *first* opening parentheses defines which part is copied to the hunk
> header.

Yes.

> Since you placed everything into parentheses (unnecessarily, I
> think),

No, not unnecessarily because otherwise only "public" for example would be copied. I agree though that leading whitespace should not be included in the parentheses.

Paolo