Subject: Re: [PATCH] Add matchings for csharp

From: Paolo Bonzini <[email-masked]>
Date: Mon, 22 Jun 2009 10:12:18 +0200

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