Subject: Re: [PATCH] Add matchings for csharp

From: Johannes Sixt <[email-masked]>
Date: Mon, 22 Jun 2009 08:24:11 +0200

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

> @@ -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