{"thread":{"id":"36503","subject":"[PATCH] Updated C# userdiff patterns.","startedAt":"2014-04-25T23:25:28Z","lastAt":"2014-04-27T20:12:44Z","messageCount":16,"participants":["Marius Ungureanu","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"239730","messageId":"29F78086-81B4-481F-9051-FF3EEBA9BB08@xamarin.com","threadId":"36503","inReplyTo":null,"subject":"[PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-25T23:25:28Z","receivedAt":"2014-04-25T23:25:28Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"New keywords: foreach, break, in, try, finally, as, is, typeof, var,\ndefault, fixed, checked, unchecked, this, lock, readonly, unsafe,\nref, out, base, null, delegate, continue.\n\nRemoved keywords: instanceof. It's only in Java.\nMoved keywords to happen before modifier parsing, as matching a keyword\nwill stop modifiers from being matched.\n\nAdded method modifiers: extern, abstract.\n\nAdded properties modifiers: abstract.\n\nAdded parsing of events and delegates, which are like properties, but\ntake an extra keyword.\n\nThe reasoning behind adding unsafe to keywords is being also a\nstatement that can happen inline in code to mention blocks which are\nunsafe. Also, delegates are not necessarily declared in class bodies,\nbut can also happen inline in other functions.\n\nKeywords are based on MSDN docs.\n\nSigned-off-by: Marius Ungureanu <marius.ungureanu@xamarin.com>\n---\n userdiff.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex fad52d6..7612c5d 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -133,14 +133,14 @@ PATTERNS(\"cpp\",\n \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n PATTERNS(\"csharp\",\n-\t /* Keywords */\n-\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n \t /* Methods and constructors */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n-\t /* Properties */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n+\t \"^[ \\t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n+\t /* Properties, events, delegates */\n+\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*((delegate|event)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n \t /* Type definitions */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n+\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n+\t /* Keywords */\n+\t \"!^[ \\t]*(as|base|break|case|catch|checked|continue|default|delegate|do|else|finally|fixed|for|foreach|if|in|is|lock|new|null|out|readonly|ref|return|switch|this|throw|try|typeof|unchecked|unsafe|using|var|while)\\n\"\n \t /* Namespace */\n \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n \t /* -- */\n-- \n1.8.4\n"},{"id":"239743","messageId":"535B5BFF.40002@kdbg.org","threadId":"36503","inReplyTo":"29F78086-81B4-481F-9051-FF3EEBA9BB08@xamarin.com","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-04-26T07:10:55Z","receivedAt":"2014-04-26T07:10:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.04.2014 01:25, schrieb Marius Ungureanu:\n> New keywords: foreach, break, in, try, finally, as, is, typeof, var,\n> default, fixed, checked, unchecked, this, lock, readonly, unsafe,\n> ref, out, base, null, delegate, continue.\n> \n> Removed keywords: instanceof. It's only in Java.\n> Moved keywords to happen before modifier parsing, as matching a keyword\n> will stop modifiers from being matched.\n> \n> Added method modifiers: extern, abstract.\n> \n> Added properties modifiers: abstract.\n> \n> Added parsing of events and delegates, which are like properties, but\n> take an extra keyword.\n> \n> The reasoning behind adding unsafe to keywords is being also a\n> statement that can happen inline in code to mention blocks which are\n> unsafe. Also, delegates are not necessarily declared in class bodies,\n> but can also happen inline in other functions.\n> \n> Keywords are based on MSDN docs.\n> \n> Signed-off-by: Marius Ungureanu <marius.ungureanu@xamarin.com>\n\nThanks for your contribution.\n\nPlease write the commit message in imperative mood, and use full\nsentences, not just fragments and avoid contractions (\"it's\"). Also,\ndon't capitalize the subject line and drop the full-stop:\n\n   update C# userdiff patterns\n\n   Add new keywords: foreach, break, ...\n\n   Remove keyword instanceof because it is only in Java. ...\n\nBTW, it is now dead easy to add test cases for userdiff patterns. Just\ndrop files with content like this into t/t4018:\n\n---- t/t4018/csharp-ignore-statement-keywords -----\nclass Foo {\n\tpublic int RIGHT()\n\t{\n\t\tif (x)\n\t\telse\n\t\ttry\n\t\tcatch (y)\n\t\t...\n\n\t\tChangeMe;\n\t}\n}\n-----------------------------------------\n\n(This I just invented, I don't do C#.) See the README file in that\ndirectory.\n\n> ---\n>  userdiff.c | 12 ++++++------\n>  1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/userdiff.c b/userdiff.c\n> index fad52d6..7612c5d 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -133,14 +133,14 @@ PATTERNS(\"cpp\",\n>  \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n>  \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n>  PATTERNS(\"csharp\",\n> -\t /* Keywords */\n> -\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n>  \t /* Methods and constructors */\n> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n> -\t /* Properties */\n> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n> +\t \"^[ \\t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n> +\t /* Properties, events, delegates */\n> +\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*((delegate|event)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>  \t /* Type definitions */\n> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n> +\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n> +\t /* Keywords */\n> +\t \"!^[ \\t]*(as|base|break|case|catch|checked|continue|default|delegate|do|else|finally|fixed|for|foreach|if|in|is|lock|new|null|out|readonly|ref|return|switch|this|throw|try|typeof|unchecked|unsafe|using|var|while)\\n\"\n>  \t /* Namespace */\n>  \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n>  \t /* -- */\n\nHere, you are moving keywords down, but in the commit message you say\nthat you \"moved keywords to happen before modifier parsing\". Aren't you\nmoving keywords *after* something? (Where the \"modifiers\" are here is\nnot obvious, but that can be attributed to that I don't do C#.)\n\nBTW, I appreciate that you re-arrange keywords alphabetically. Could you\ndo that in the commit message, too?\n\n-- Hannes\n"},{"id":"239750","messageId":"97789B23-A375-46B1-99FD-A851A15C2D85@xamarin.com","threadId":"36503","inReplyTo":"535B5BFF.40002@kdbg.org","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-26T09:55:10Z","receivedAt":"2014-04-26T09:55:10Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"\nOn 26 Apr 2014, at 10:10, Johannes Sixt <j6t@kdbg.org> wrote:\n\n> Am 26.04.2014 01:25, schrieb Marius Ungureanu:\n>> New keywords: foreach, break, in, try, finally, as, is, typeof, var,\n>> default, fixed, checked, unchecked, this, lock, readonly, unsafe,\n>> ref, out, base, null, delegate, continue.\n>> \n>> Removed keywords: instanceof. It's only in Java.\n>> Moved keywords to happen before modifier parsing, as matching a keyword\n>> will stop modifiers from being matched.\n>> \n>> Added method modifiers: extern, abstract.\n>> \n>> Added properties modifiers: abstract.\n>> \n>> Added parsing of events and delegates, which are like properties, but\n>> take an extra keyword.\n>> \n>> The reasoning behind adding unsafe to keywords is being also a\n>> statement that can happen inline in code to mention blocks which are\n>> unsafe. Also, delegates are not necessarily declared in class bodies,\n>> but can also happen inline in other functions.\n>> \n>> Keywords are based on MSDN docs.\n>> \n>> Signed-off-by: Marius Ungureanu <marius.ungureanu@xamarin.com>\n> \n> Thanks for your contribution.\n> \n> Please write the commit message in imperative mood, and use full\n> sentences, not just fragments and avoid contractions (\"it's\"). Also,\n> don't capitalize the subject line and drop the full-stop:\n> \n>   update C# userdiff patterns\n> \n>   Add new keywords: foreach, break, ...\n> \n>   Remove keyword instanceof because it is only in Java. …\n> \n\nHey!\n\nI’ll fix the commit message and description.\n\n> BTW, it is now dead easy to add test cases for userdiff patterns. Just\n> drop files with content like this into t/t4018:\n> \n> ---- t/t4018/csharp-ignore-statement-keywords -----\n> class Foo {\n> \tpublic int RIGHT()\n> \t{\n> \t\tif (x)\n> \t\telse\n> \t\ttry\n> \t\tcatch (y)\n> \t\t...\n> \n> \t\tChangeMe;\n> \t}\n> }\n> ————————————————————\n> \n\nGreat, I’ll make another commit with adding unit tests. Thanks!\n\n> (This I just invented, I don't do C#.) See the README file in that\n> directory.\n> \n>> ---\n>> userdiff.c | 12 ++++++------\n>> 1 file changed, 6 insertions(+), 6 deletions(-)\n>> \n>> diff --git a/userdiff.c b/userdiff.c\n>> index fad52d6..7612c5d 100644\n>> --- a/userdiff.c\n>> +++ b/userdiff.c\n>> @@ -133,14 +133,14 @@ PATTERNS(\"cpp\",\n>> \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n>> \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n>> PATTERNS(\"csharp\",\n>> -\t /* Keywords */\n>> -\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n>> \t /* Methods and constructors */\n>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>> -\t /* Properties */\n>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>> +\t \"^[ \\t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>> +\t /* Properties, events, delegates */\n>> +\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*((delegate|event)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>> \t /* Type definitions */\n>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>> +\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>> +\t /* Keywords */\n>> +\t \"!^[ \\t]*(as|base|break|case|catch|checked|continue|default|delegate|do|else|finally|fixed|for|foreach|if|in|is|lock|new|null|out|readonly|ref|return|switch|this|throw|try|typeof|unchecked|unsafe|using|var|while)\\n\"\n>> \t /* Namespace */\n>> \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n>> \t /* -- */\n> \n> Here, you are moving keywords down, but in the commit message you say\n> that you \"moved keywords to happen before modifier parsing\". Aren't you\n> moving keywords *after* something? (Where the \"modifiers\" are here is\n> not obvious, but that can be attributed to that I don't do C#.)\n> \n\nIt was a typo because I was sleepy. It was intentional to move them *after*.\nModifier parsing can contain keywords, so just to be sure, I moved the\nkeywords after modifier parsing, so it uses the keywords as a fallback.\nIf this is not what should happen, please tell.\n\nModifiers are prefixes to methods/properties. (the pipe separated lists)\n\n> BTW, I appreciate that you re-arrange keywords alphabetically. Could you\n> do that in the commit message, too?\n> \n> — Hannes\n\nI’ll handle this too, no problem.\n\nThanks,\nMarius\n"},{"id":"239751","messageId":"2E60EB9E-E06B-4A5C-AC97-38847F300CAC@xamarin.com","threadId":"36503","inReplyTo":"97789B23-A375-46B1-99FD-A851A15C2D85@xamarin.com","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-26T10:49:44Z","receivedAt":"2014-04-26T10:49:44Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"On a side note, I noticed some of the keywords I added shouldn’t be there.\n\nI just realised that simple statements have no reason to be there, but only\nblock definitions. I’ll reduce the size of this patch on the keywords part.\n\nThanks,\nMarius"},{"id":"239767","messageId":"535BF1C0.7080204@kdbg.org","threadId":"36503","inReplyTo":"97789B23-A375-46B1-99FD-A851A15C2D85@xamarin.com","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-04-26T17:49:52Z","receivedAt":"2014-04-26T17:49:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.04.2014 11:55, schrieb Marius Ungureanu:\n> On 26 Apr 2014, at 10:10, Johannes Sixt <j6t@kdbg.org> wrote:\n>> Am 26.04.2014 01:25, schrieb Marius Ungureanu:\n>>> diff --git a/userdiff.c b/userdiff.c\n>>> index fad52d6..7612c5d 100644\n>>> --- a/userdiff.c\n>>> +++ b/userdiff.c\n>>> @@ -133,14 +133,14 @@ PATTERNS(\"cpp\",\n>>> \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n>>> \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n>>> PATTERNS(\"csharp\",\n>>> -\t /* Keywords */\n>>> -\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n>>> \t /* Methods and constructors */\n>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>>> -\t /* Properties */\n>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>>> +\t \"^[ \\t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>>> +\t /* Properties, events, delegates */\n>>> +\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*((delegate|event)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>>> \t /* Type definitions */\n>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>>> +\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>>> +\t /* Keywords */\n>>> +\t \"!^[ \\t]*(as|base|break|case|catch|checked|continue|default|delegate|do|else|finally|fixed|for|foreach|if|in|is|lock|new|null|out|readonly|ref|return|switch|this|throw|try|typeof|unchecked|unsafe|using|var|while)\\n\"\n>>> \t /* Namespace */\n>>> \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n>>> \t /* -- */\n> \n> ...\n> Modifier parsing can contain keywords, so just to be sure, I moved the\n> keywords after modifier parsing, so it uses the keywords as a fallback.\n> If this is not what should happen, please tell.\n\nFor each line, patterns are are scanned in order, and the first match\ndetermines the outcome: If it is a negative pattern (i.e., it begins\nwith an exclamation mark), the line is not a hunk header; if it is a\npositive pattern, the line is a hunk header. If no pattern matches, the\nline is not a hunk header, either; it is as if the list were terminated\nby a negative catch-all pattern.\n\nDue to these rules, negative patterns in the list are only necessary\nwhen you want to make an exception to a positive pattern in the list,\nand then the negative pattern must be listed before the positive pattern.\n\nIn the csharp case, I do not see a pattern of which the keyword pattern\nwould make an exception (neither the old version nor your new version).\nTherefore, you could drop the keyword pattern entirely.\n\n-- Hannes\n"},{"id":"239773","messageId":"1EA6F663-54DC-4665-A88C-8627F38B356E@xamarin.com","threadId":"36503","inReplyTo":"535BF1C0.7080204@kdbg.org","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-26T18:33:52Z","receivedAt":"2014-04-26T18:33:52Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"On 26 Apr 2014, at 20:49, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 26.04.2014 11:55, schrieb Marius Ungureanu:\n>> On 26 Apr 2014, at 10:10, Johannes Sixt <j6t@kdbg.org> wrote:\n>>> Am 26.04.2014 01:25, schrieb Marius Ungureanu:\n>>>> diff --git a/userdiff.c b/userdiff.c\n>>>> index fad52d6..7612c5d 100644\n>>>> --- a/userdiff.c\n>>>> +++ b/userdiff.c\n>>>> @@ -133,14 +133,14 @@ PATTERNS(\"cpp\",\n>>>> \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n>>>> \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n>>>> PATTERNS(\"csharp\",\n>>>> -\t /* Keywords */\n>>>> -\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n>>>> \t /* Methods and constructors */\n>>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>>>> -\t /* Properties */\n>>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>>>> +\t \"^[ \\t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n>>>> +\t /* Properties, events, delegates */\n>>>> +\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*((delegate|event)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n>>>> \t /* Type definitions */\n>>>> -\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>>>> +\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n>>>> +\t /* Keywords */\n>>>> +\t \"!^[ \\t]*(as|base|break|case|catch|checked|continue|default|delegate|do|else|finally|fixed|for|foreach|if|in|is|lock|new|null|out|readonly|ref|return|switch|this|throw|try|typeof|unchecked|unsafe|using|var|while)\\n\"\n>>>> \t /* Namespace */\n>>>> \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n>>>> \t /* -- */\n>> \n>> ...\n>> Modifier parsing can contain keywords, so just to be sure, I moved the\n>> keywords after modifier parsing, so it uses the keywords as a fallback.\n>> If this is not what should happen, please tell.\n> \n> For each line, patterns are are scanned in order, and the first match\n> determines the outcome: If it is a negative pattern (i.e., it begins\n> with an exclamation mark), the line is not a hunk header; if it is a\n> positive pattern, the line is a hunk header. If no pattern matches, the\n> line is not a hunk header, either; it is as if the list were terminated\n> by a negative catch-all pattern.\n> \n> Due to these rules, negative patterns in the list are only necessary\n> when you want to make an exception to a positive pattern in the list,\n> and then the negative pattern must be listed before the positive pattern.\n> \n> In the csharp case, I do not see a pattern of which the keyword pattern\n> would make an exception (neither the old version nor your new version).\n> Therefore, you could drop the keyword pattern entirely.\n> \n> -- Hannes\n> \n\nHey!\n\nI’ll remove them and add as many unit tests I can. I’ve been side tracked\ntoday and I couldn’t get to look at it. I’ll start a new thread with the new\npatch as soon as I’m done with it.\n\nThanks for all the tips until now. You’ve been of great help.\n\nMarius"},{"id":"239774","messageId":"535BFFEB.8040103@kdbg.org","threadId":"36503","inReplyTo":"1EA6F663-54DC-4665-A88C-8627F38B356E@xamarin.com","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-04-26T18:50:19Z","receivedAt":"2014-04-26T18:50:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.04.2014 20:33, schrieb Marius Ungureanu:\n> ... add as many unit tests I can.\n\nGreat! Keep in mind that quantity is secondary. Quality counts.\n\n> I’ll start a new thread with the new\n> patch as soon as I’m done with it.\n\nIf possible, do not start a new thread, but post your new patch as a\nreply in this thread.\n\nThanks,\n-- Hannes\n"},{"id":"239795","messageId":"E0FA908C-1D48-401C-8865-EA9AD8B0531E@xamarin.com","threadId":"36503","inReplyTo":"535BFFEB.8040103@kdbg.org","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-26T18:52:17Z","receivedAt":"2014-04-26T18:52:17Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"\nOn 26 Apr 2014, at 21:50, Johannes Sixt <j6t@kdbg.org> wrote:\n\n> Am 26.04.2014 20:33, schrieb Marius Ungureanu:\n>> ... add as many unit tests I can.\n> \n> Great! Keep in mind that quantity is secondary. Quality counts.\n\nObviously. By ‘as many’, I meant to cover all the cases here.\n\n>> I’ll start a new thread with the new\n>> patch as soon as I’m done with it.\n> \n> If possible, do not start a new thread, but post your new patch as a\n> reply in this thread.\n> \n\nOkay, understood.\n\n> Thanks,\n> -- Hannes\n> \n\nThanks,\nMarius\n"},{"id":"239796","messageId":"241D6B37-A274-4425-868C-E15651132B24@xamarin.com","threadId":"36503","inReplyTo":"535BFFEB.8040103@kdbg.org","subject":"Re: [PATCH] Updated C# userdiff patterns.","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T13:43:49Z","receivedAt":"2014-04-27T13:43:49Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"Heya, so I’ll add the patches in the next 2 emails.\n\nI’ve changed a bit the main body of the methods/constructors regex.\n\nBasically, I’ve made the first item after the modifiers optional. That’s\nthe return type and it’s not used in any case by operator overloads\nor constructors/destructors.\n\nI also added lots of symbols to the name of the function. Those are\nthe symbols of the operators that the language allows you to overload.\n\nThanks in advance,\nMarius."},{"id":"239797","messageId":"31BBCA60-8EC8-43B1-B82F-071324036AC8@xamarin.com","threadId":"36503","inReplyTo":"535BFFEB.8040103@kdbg.org","subject":"Re: [PATCH 1/2] update C# userdiff patterns","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T13:45:46Z","receivedAt":"2014-04-27T13:45:46Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"Remove all keywords since they don't cause conflicts.\n\nAdd method modifiers: abstract, async, explicit, extern, implicit,\npartial, operator.\n\nAdd properties modifiers: abstract, readonly.\n\nAdd type modifiers: abstract.\n\nParse C# operator functions.\n\nFix constructor parsing.\n\nSigned-off-by: Marius Ungureanu <marius.ungureanu@xamarin.com>\n---\n userdiff.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex fad52d6..e7ba5e3 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -133,14 +133,12 @@ PATTERNS(\"cpp\",\n \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*\"\n \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\\\\*?|\\\\.\\\\*\"),\n PATTERNS(\"csharp\",\n-\t /* Keywords */\n-\t \"!^[ \\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\\n\"\n \t /* Methods and constructors */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[<>@._[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n+\t \"^[ \\t]*(((abstract|async|explicit|extern|implicit|internal|new|operator|override|partial|private|protected|public|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@._[:alnum:]]*[ \\t]*[-<>@.~=+!*%&|^_[:alnum:]]+[ \\t]*\\\\(.*\\\\))[ \\t]*$\\n\"\n \t /* Properties */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n+\t \"^[ \\t]*(((abstract|internal|new|override|private|protected|public|readonly|sealed|static|unsafe|virtual)[ \\t]+)*[][<>@.~_[:alnum:]]+[ \\t]+[@._[:alnum:]]+)[ \\t]*$\\n\"\n \t /* Type definitions */\n-\t \"^[ \\t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n+\t \"^[ \\t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \\t]+)*(class|enum|interface|struct)[ \\t]+.*)$\\n\"\n \t /* Namespace */\n \t \"^[ \\t]*(namespace[ \\t]+.*)$\",\n \t /* -- */\n-- \n1.8.4\n"},{"id":"239798","messageId":"3A0D05C9-C222-463E-BCD4-CD38F216E352@xamarin.com","threadId":"36503","inReplyTo":"535BFFEB.8040103@kdbg.org","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T13:47:01Z","receivedAt":"2014-04-27T13:47:01Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"\n---\n t/t4018/csharp-constructor | 4 ++++\n t/t4018/csharp-destructor  | 4 ++++\n t/t4018/csharp-function    | 4 ++++\n t/t4018/csharp-member      | 4 ++++\n t/t4018/csharp-namespace   | 4 ++++\n t/t4018/csharp-operator    | 4 ++++\n t/t4018/csharp-property    | 4 ++++\n t/t4018/csharp-skip-call   | 5 +++++\n 8 files changed, 33 insertions(+)\n create mode 100644 t/t4018/csharp-constructor\n create mode 100644 t/t4018/csharp-destructor\n create mode 100644 t/t4018/csharp-function\n create mode 100644 t/t4018/csharp-member\n create mode 100644 t/t4018/csharp-namespace\n create mode 100644 t/t4018/csharp-operator\n create mode 100644 t/t4018/csharp-property\n create mode 100644 t/t4018/csharp-skip-call\n\ndiff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor\nnew file mode 100644\nindex 0000000..a39cffb\n--- /dev/null\n+++ b/t/t4018/csharp-constructor\n@@ -0,0 +1,4 @@\n+MyClass(RIGHT)\n+{\n+\tChangeMe;\n+}\ndiff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor\nnew file mode 100644\nindex 0000000..55106d9\n--- /dev/null\n+++ b/t/t4018/csharp-destructor\n@@ -0,0 +1,4 @@\n+~MyClass(RIGHT)\n+{\n+\tChangeMe;\n+}\ndiff --git a/t/t4018/csharp-function b/t/t4018/csharp-function\nnew file mode 100644\nindex 0000000..a5d59a3\n--- /dev/null\n+++ b/t/t4018/csharp-function\n@@ -0,0 +1,4 @@\n+virtual DoStuff(RIGHT)\n+{\n+\tChangeMe;\n+}\ndiff --git a/t/t4018/csharp-member b/t/t4018/csharp-member\nnew file mode 100644\nindex 0000000..4939d53\n--- /dev/null\n+++ b/t/t4018/csharp-member\n@@ -0,0 +1,4 @@\n+unsafe class RIGHT\n+{\n+\tint ChangeMe;\n+}\ndiff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace\nnew file mode 100644\nindex 0000000..6749980\n--- /dev/null\n+++ b/t/t4018/csharp-namespace\n@@ -0,0 +1,4 @@\n+namespace RIGHT\n+{\n+\tChangeMe;\n+}\ndiff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator\nnew file mode 100644\nindex 0000000..5b00263\n--- /dev/null\n+++ b/t/t4018/csharp-operator\n@@ -0,0 +1,4 @@\n+A operator+(RIGHT)\n+{\n+\tChangeMe;\n+}\ndiff --git a/t/t4018/csharp-property b/t/t4018/csharp-property\nnew file mode 100644\nindex 0000000..82d67fc\n--- /dev/null\n+++ b/t/t4018/csharp-property\n@@ -0,0 +1,4 @@\n+public virtual int RIGHT\n+{\n+\tget { ChangeMe; }\n+}\ndiff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call\nnew file mode 100644\nindex 0000000..e89d083\n--- /dev/null\n+++ b/t/t4018/csharp-skip-call\n@@ -0,0 +1,5 @@\n+virtual void RIGHT()\n+{\n+\tcall();\n+\tChangeMe;\n+}\n-- \n1.8.4\n"},{"id":"239799","messageId":"1D0349EA-542F-46BD-B538-65ACD0A46A81@xamarin.com","threadId":"36503","inReplyTo":"31BBCA60-8EC8-43B1-B82F-071324036AC8@xamarin.com","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T13:48:14Z","receivedAt":"2014-04-27T13:48:14Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"Ugh, there’s an empty line at the beginning of the 2nd patch that shouldn’t be there.\n\nThanks,\nMarius"},{"id":"239801","messageId":"535D2E0C.40101@kdbg.org","threadId":"36503","inReplyTo":"3A0D05C9-C222-463E-BCD4-CD38F216E352@xamarin.com","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-04-27T16:19:24Z","receivedAt":"2014-04-27T16:19:24Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.04.2014 15:47, schrieb Marius Ungureanu:\n> \n> ---\n\nThanks. Please signed off your patch.\n\nWhen you re-send, please place [PATCH v3 n/m] in the subject (and drop\nthe \"Re:\") and note what you changed compared to the previous (or all\nearlier) rounds. The place for this note is here, after the \"---\" marker.\n\nHave a look at Documentation/SubmittingPatches.\n\n>  t/t4018/csharp-constructor | 4 ++++\n>  t/t4018/csharp-destructor  | 4 ++++\n>  t/t4018/csharp-function    | 4 ++++\n>  t/t4018/csharp-member      | 4 ++++\n>  t/t4018/csharp-namespace   | 4 ++++\n>  t/t4018/csharp-operator    | 4 ++++\n>  t/t4018/csharp-property    | 4 ++++\n>  t/t4018/csharp-skip-call   | 5 +++++\n>  8 files changed, 33 insertions(+)\n>  create mode 100644 t/t4018/csharp-constructor\n>  create mode 100644 t/t4018/csharp-destructor\n>  create mode 100644 t/t4018/csharp-function\n>  create mode 100644 t/t4018/csharp-member\n>  create mode 100644 t/t4018/csharp-namespace\n>  create mode 100644 t/t4018/csharp-operator\n>  create mode 100644 t/t4018/csharp-property\n>  create mode 100644 t/t4018/csharp-skip-call\n> \n\nUnfortunately, I think you have reduced the test cases too far. One of\nthe main properties of C# code is that usually member and property\ndefinitions are indented and there is a class definition around them:\n\n  class Foo {\n     Foo(X) {}\n     virtual void DoStuff() {}\n     public int X;\n  };\n\nIn your examples, you omitted the surrounding class definition and did\nnot indent the member definitions. By doing so, the test cases do not\ndemonstrate that the csharp userdiff patterns are significantly\ndifferent from the default userdiff pattern: in the examples you\npresent, the default pattern would have picked the same hunk headers as\nthe csharp patterns!\n\nFor a reviewer who is not (or only marginally) familiar with C# (like\nmyself), it would have been very instructive to present patches with\ntest cases that demonstrate weaknesses of the old patterns before\npatches that fix them. For example, you say that you fix the constructor\npattern. But I am unable to judge what is wrong and how you fix it. The\ncommit message is the right place to add text that helps reviewers.\n\nYou can mark a userdiff test case that demonstrates a breakage by\nincluding the work \"broken\" somewhere in the file. See\nhttp://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046\n\n> diff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor\n> new file mode 100644\n> index 0000000..a39cffb\n> --- /dev/null\n> +++ b/t/t4018/csharp-constructor\n> @@ -0,0 +1,4 @@\n> +MyClass(RIGHT)\n> +{\n> +\tChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor\n> new file mode 100644\n> index 0000000..55106d9\n> --- /dev/null\n> +++ b/t/t4018/csharp-destructor\n> @@ -0,0 +1,4 @@\n> +~MyClass(RIGHT)\n> +{\n> +\tChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-function b/t/t4018/csharp-function\n> new file mode 100644\n> index 0000000..a5d59a3\n> --- /dev/null\n> +++ b/t/t4018/csharp-function\n> @@ -0,0 +1,4 @@\n> +virtual DoStuff(RIGHT)\n> +{\n> +\tChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-member b/t/t4018/csharp-member\n> new file mode 100644\n> index 0000000..4939d53\n> --- /dev/null\n> +++ b/t/t4018/csharp-member\n> @@ -0,0 +1,4 @@\n> +unsafe class RIGHT\n> +{\n> +\tint ChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace\n> new file mode 100644\n> index 0000000..6749980\n> --- /dev/null\n> +++ b/t/t4018/csharp-namespace\n> @@ -0,0 +1,4 @@\n> +namespace RIGHT\n> +{\n> +\tChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator\n> new file mode 100644\n> index 0000000..5b00263\n> --- /dev/null\n> +++ b/t/t4018/csharp-operator\n\n\"csharp-user-defined-operator\" would more precisely describe the case. I\nwouldn't mind seening other file names being a bit more elaborate, but I\nfind this one particularly ambiguous.\n\n> @@ -0,0 +1,4 @@\n> +A operator+(RIGHT)\n> +{\n> +\tChangeMe;\n> +}\n> diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property\n> new file mode 100644\n> index 0000000..82d67fc\n> --- /dev/null\n> +++ b/t/t4018/csharp-property\n> @@ -0,0 +1,4 @@\n> +public virtual int RIGHT\n> +{\n> +\tget { ChangeMe; }\n> +}\n> diff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call\n> new file mode 100644\n> index 0000000..e89d083\n> --- /dev/null\n> +++ b/t/t4018/csharp-skip-call\n> @@ -0,0 +1,5 @@\n> +virtual void RIGHT()\n> +{\n> +\tcall();\n> +\tChangeMe;\n> +}\n\nIn this last test case, you want to demonstrate that the line \"call()\"\nis not picked as hunk header. As written, the line would never be picked\nas hunk header, even if it would match some pattern, because it is too\nclose to \"ChangeMe\". You must have at least one more line between\n\"call()\" and \"ChangeMe\".\n\nBTW, what is the expected hunk header in a diff like the following where\n\"class Foo\" is at line 1 in the file (just above the hunk)?\n\n@@ -2,3 +2,3 @@\n {\n-   // old comment\n+   // new comment\n    public whatever()\n\nThat is, when the class definition is undecorated (no \"unsafe\" etc.)\n\n-- Hannes\n"},{"id":"239802","messageId":"CF7066B0-539D-4FAB-8845-8ED60DF1F17E@xamarin.com","threadId":"36503","inReplyTo":"535D2E0C.40101@kdbg.org","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T16:46:30Z","receivedAt":"2014-04-27T16:46:30Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"\nOn 27 Apr 2014, at 19:19, Johannes Sixt <j6t@kdbg.org> wrote:\n\n> Am 27.04.2014 15:47, schrieb Marius Ungureanu:\n>> \n>> ---\n> \n> Thanks. Please signed off your patch.\n> \n\nAh, yes, forgot to do that.\n\n> When you re-send, please place [PATCH v3 n/m] in the subject (and drop\n> the \"Re:\") and note what you changed compared to the previous (or all\n> earlier) rounds. The place for this note is here, after the \"---\" marker.\n> \n> Have a look at Documentation/SubmittingPatches.\n> \n\nWill do so.\n\n>> t/t4018/csharp-constructor | 4 ++++\n>> t/t4018/csharp-destructor  | 4 ++++\n>> t/t4018/csharp-function    | 4 ++++\n>> t/t4018/csharp-member      | 4 ++++\n>> t/t4018/csharp-namespace   | 4 ++++\n>> t/t4018/csharp-operator    | 4 ++++\n>> t/t4018/csharp-property    | 4 ++++\n>> t/t4018/csharp-skip-call   | 5 +++++\n>> 8 files changed, 33 insertions(+)\n>> create mode 100644 t/t4018/csharp-constructor\n>> create mode 100644 t/t4018/csharp-destructor\n>> create mode 100644 t/t4018/csharp-function\n>> create mode 100644 t/t4018/csharp-member\n>> create mode 100644 t/t4018/csharp-namespace\n>> create mode 100644 t/t4018/csharp-operator\n>> create mode 100644 t/t4018/csharp-property\n>> create mode 100644 t/t4018/csharp-skip-call\n>> \n> \n> Unfortunately, I think you have reduced the test cases too far. One of\n> the main properties of C# code is that usually member and property\n> definitions are indented and there is a class definition around them:\n> \n>  class Foo {\n>     Foo(X) {}\n>     virtual void DoStuff() {}\n>     public int X;\n>  };\n> \n> In your examples, you omitted the surrounding class definition and did\n> not indent the member definitions. By doing so, the test cases do not\n> demonstrate that the csharp userdiff patterns are significantly\n> different from the default userdiff pattern: in the examples you\n> present, the default pattern would have picked the same hunk headers as\n> the csharp patterns!\n> \n\nAh, I think I over judged minimal sample here. I’ll do so.\n\nIs it okay though if I add a few tests to show what is broken?\n\nI think this can’t be solved at a regex level.\n\n> For a reviewer who is not (or only marginally) familiar with C# (like\n> myself), it would have been very instructive to present patches with\n> test cases that demonstrate weaknesses of the old patterns before\n> patches that fix them. For example, you say that you fix the constructor\n> pattern. But I am unable to judge what is wrong and how you fix it. The\n> commit message is the right place to add text that helps reviewers.\n> \n\nWell, the previous pattern didn’t match constructors as they should be\nat a logical level. That means, it considered the constructor name\nas being the return type. It’s just a logical change that helped with\nwriting operator function parsing.\n\n> You can mark a userdiff test case that demonstrates a breakage by\n> including the work \"broken\" somewhere in the file. See\n> http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046\n> \n>> diff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor\n>> new file mode 100644\n>> index 0000000..a39cffb\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-constructor\n>> @@ -0,0 +1,4 @@\n>> +MyClass(RIGHT)\n>> +{\n>> +\tChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor\n>> new file mode 100644\n>> index 0000000..55106d9\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-destructor\n>> @@ -0,0 +1,4 @@\n>> +~MyClass(RIGHT)\n>> +{\n>> +\tChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-function b/t/t4018/csharp-function\n>> new file mode 100644\n>> index 0000000..a5d59a3\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-function\n>> @@ -0,0 +1,4 @@\n>> +virtual DoStuff(RIGHT)\n>> +{\n>> +\tChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-member b/t/t4018/csharp-member\n>> new file mode 100644\n>> index 0000000..4939d53\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-member\n>> @@ -0,0 +1,4 @@\n>> +unsafe class RIGHT\n>> +{\n>> +\tint ChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace\n>> new file mode 100644\n>> index 0000000..6749980\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-namespace\n>> @@ -0,0 +1,4 @@\n>> +namespace RIGHT\n>> +{\n>> +\tChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator\n>> new file mode 100644\n>> index 0000000..5b00263\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-operator\n> \n> \"csharp-user-defined-operator\" would more precisely describe the case. I\n> wouldn't mind seening other file names being a bit more elaborate, but I\n> find this one particularly ambiguous.\n> \n\nOkay, I’ll name them better. Also improve them.\n\n>> @@ -0,0 +1,4 @@\n>> +A operator+(RIGHT)\n>> +{\n>> +\tChangeMe;\n>> +}\n>> diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property\n>> new file mode 100644\n>> index 0000000..82d67fc\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-property\n>> @@ -0,0 +1,4 @@\n>> +public virtual int RIGHT\n>> +{\n>> +\tget { ChangeMe; }\n>> +}\n>> diff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call\n>> new file mode 100644\n>> index 0000000..e89d083\n>> --- /dev/null\n>> +++ b/t/t4018/csharp-skip-call\n>> @@ -0,0 +1,5 @@\n>> +virtual void RIGHT()\n>> +{\n>> +\tcall();\n>> +\tChangeMe;\n>> +}\n> \n> In this last test case, you want to demonstrate that the line \"call()\"\n> is not picked as hunk header. As written, the line would never be picked\n> as hunk header, even if it would match some pattern, because it is too\n> close to \"ChangeMe\". You must have at least one more line between\n> \"call()\" and \"ChangeMe”.\n> \n\nOh, forgot about that.\n\n> BTW, what is the expected hunk header in a diff like the following where\n> \"class Foo\" is at line 1 in the file (just above the hunk)?\n> \n> @@ -2,3 +2,3 @@\n> {\n> -   // old comment\n> +   // new comment\n>    public whatever()\n> \n> That is, when the class definition is undecorated (no \"unsafe\" etc.)\n\nThe class name should still get matched in that case even if it isn’t decorated.\n\n> \n> -- Hannes\n\nMarius"},{"id":"239803","messageId":"33F87557-C685-4338-9BFA-2391442CD7A1@xamarin.com","threadId":"36503","inReplyTo":"535D2E0C.40101@kdbg.org","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Marius Ungureanu","fromEmail":"marius.ungureanu@xamarin.com","sentAt":"2014-04-27T17:11:30Z","receivedAt":"2014-04-27T17:11:30Z","isPatch":true,"sender":{"key":"marius.ungureanu@xamarin.com","avatar":"https://gravatar.com/avatar/b877b5245eb7e82cc4537dce4a5f6733d120a2502fe6ec49499d622b1f785745?d=mp&s=160"},"body":"\n> You can mark a userdiff test case that demonstrates a breakage by\n> including the work \"broken\" somewhere in the file. See\n> http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046\n> -- Hannes\n\nWould you like me to add tests first, then fix them? Or just like this?"},{"id":"239819","messageId":"535D64BC.8060403@kdbg.org","threadId":"36503","inReplyTo":"CF7066B0-539D-4FAB-8845-8ED60DF1F17E@xamarin.com","subject":"Re: [PATCH 2/2] add csharp userdiff tests","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-04-27T20:12:44Z","receivedAt":"2014-04-27T20:12:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.04.2014 18:46, schrieb Marius Ungureanu:\n> Is it okay though if I add a few tests to show what is broken?\n> \n> I think this can’t be solved at a regex level.\n\nIt's OK to add tests that show breakages even if there is no immediate\nsolution.\n\n>> You can mark a userdiff test case that demonstrates a breakage by\n>> including the work \"broken\" somewhere in the file. See\n>> http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046\n\nYou add tests including broken cases first, and then in the follow-up\npatch that fixes the broken ones, you also mark the tests as fixed, like\nI did in the follow-up patch of the above example:\nhttp://www.repo.or.cz/w/alt-git.git/commitdiff/8a2e8da367f7175465118510b474ad365161d6b1\n\n-- Hannes\n"}]}