Re: [PATCH] Updated C# userdiff patterns.

2014-04-27 Thread Marius Ungureanu 

On 26 Apr 2014, at 21:50, Johannes Sixt j...@kdbg.org wrote:

 Am 26.04.2014 20:33, schrieb Marius Ungureanu:
 ... add as many unit tests I can.
 
 Great! Keep in mind that quantity is secondary. Quality counts.

Obviously. By ‘as many’, I meant to cover all the cases here.

 I’ll start a new thread with the new
 patch as soon as I’m done with it.
 
 If possible, do not start a new thread, but post your new patch as a
 reply in this thread.
 

Okay, understood.

 Thanks,
 -- Hannes
 

Thanks,
Marius

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-27 Thread Marius Ungureanu 
Heya, so I’ll add the patches in the next 2 emails.

I’ve changed a bit the main body of the methods/constructors regex.

Basically, I’ve made the first item after the modifiers optional. That’s
the return type and it’s not used in any case by operator overloads
or constructors/destructors.

I also added lots of symbols to the name of the function. Those are
the symbols of the operators that the language allows you to overload.

Thanks in advance,
Marius.--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Johannes Sixt 
Am 26.04.2014 01:25, schrieb Marius Ungureanu:
 New keywords: foreach, break, in, try, finally, as, is, typeof, var,
 default, fixed, checked, unchecked, this, lock, readonly, unsafe,
 ref, out, base, null, delegate, continue.
 
 Removed keywords: instanceof. It's only in Java.
 Moved keywords to happen before modifier parsing, as matching a keyword
 will stop modifiers from being matched.
 
 Added method modifiers: extern, abstract.
 
 Added properties modifiers: abstract.
 
 Added parsing of events and delegates, which are like properties, but
 take an extra keyword.
 
 The reasoning behind adding unsafe to keywords is being also a
 statement that can happen inline in code to mention blocks which are
 unsafe. Also, delegates are not necessarily declared in class bodies,
 but can also happen inline in other functions.
 
 Keywords are based on MSDN docs.
 
 Signed-off-by: Marius Ungureanu marius.ungure...@xamarin.com

Thanks for your contribution.

Please write the commit message in imperative mood, and use full
sentences, not just fragments and avoid contractions (it's). Also,
don't capitalize the subject line and drop the full-stop:

   update C# userdiff patterns

   Add new keywords: foreach, break, ...

   Remove keyword instanceof because it is only in Java. ...

BTW, it is now dead easy to add test cases for userdiff patterns. Just
drop files with content like this into t/t4018:

 t/t4018/csharp-ignore-statement-keywords -
class Foo {
public int RIGHT()
{
if (x)
else
try
catch (y)
...

ChangeMe;
}
}
-

(This I just invented, I don't do C#.) See the README file in that
directory.

 ---
  userdiff.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/userdiff.c b/userdiff.c
 index fad52d6..7612c5d 100644
 --- a/userdiff.c
 +++ b/userdiff.c
 @@ -133,14 +133,14 @@ PATTERNS(cpp,
|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*
|[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
  PATTERNS(csharp,
 -  /* Keywords */
 -  !^[ 
 \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
/* Methods and constructors */
 -  ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 -  /* Properties */
 -  ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
 +  ^[ 
 \t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 +  /* Properties, events, delegates */
 +  ^[ 
 \t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*((delegate|event)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ 
 \t]*$\n
/* Type definitions */
 -  ^[ 
 \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +  ^[ 
 \t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +  /* Keywords */
 +  !^[ 
 \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
/* Namespace */
^[ \t]*(namespace[ \t]+.*)$,
/* -- */

Here, you are moving keywords down, but in the commit message you say
that you moved keywords to happen before modifier parsing. Aren't you
moving keywords *after* something? (Where the modifiers are here is
not obvious, but that can be attributed to that I don't do C#.)

BTW, I appreciate that you re-arrange keywords alphabetically. Could you
do that in the commit message, too?

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Marius Ungureanu 

On 26 Apr 2014, at 10:10, Johannes Sixt j...@kdbg.org wrote:

 Am 26.04.2014 01:25, schrieb Marius Ungureanu:
 New keywords: foreach, break, in, try, finally, as, is, typeof, var,
 default, fixed, checked, unchecked, this, lock, readonly, unsafe,
 ref, out, base, null, delegate, continue.
 
 Removed keywords: instanceof. It's only in Java.
 Moved keywords to happen before modifier parsing, as matching a keyword
 will stop modifiers from being matched.
 
 Added method modifiers: extern, abstract.
 
 Added properties modifiers: abstract.
 
 Added parsing of events and delegates, which are like properties, but
 take an extra keyword.
 
 The reasoning behind adding unsafe to keywords is being also a
 statement that can happen inline in code to mention blocks which are
 unsafe. Also, delegates are not necessarily declared in class bodies,
 but can also happen inline in other functions.
 
 Keywords are based on MSDN docs.
 
 Signed-off-by: Marius Ungureanu marius.ungure...@xamarin.com
 
 Thanks for your contribution.
 
 Please write the commit message in imperative mood, and use full
 sentences, not just fragments and avoid contractions (it's). Also,
 don't capitalize the subject line and drop the full-stop:
 
   update C# userdiff patterns
 
   Add new keywords: foreach, break, ...
 
   Remove keyword instanceof because it is only in Java. …
 

Hey!

I’ll fix the commit message and description.

 BTW, it is now dead easy to add test cases for userdiff patterns. Just
 drop files with content like this into t/t4018:
 
  t/t4018/csharp-ignore-statement-keywords -
 class Foo {
   public int RIGHT()
   {
   if (x)
   else
   try
   catch (y)
   ...
 
   ChangeMe;
   }
 }
 
 

Great, I’ll make another commit with adding unit tests. Thanks!

 (This I just invented, I don't do C#.) See the README file in that
 directory.
 
 ---
 userdiff.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/userdiff.c b/userdiff.c
 index fad52d6..7612c5d 100644
 --- a/userdiff.c
 +++ b/userdiff.c
 @@ -133,14 +133,14 @@ PATTERNS(cpp,
   |[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*
   |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 - /* Keywords */
 - !^[ 
 \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
   /* Methods and constructors */
 - ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 - /* Properties */
 - ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
 + ^[ 
 \t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 + /* Properties, events, delegates */
 + ^[ 
 \t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*((delegate|event)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ 
 \t]*$\n
   /* Type definitions */
 - ^[ 
 \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 + ^[ 
 \t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 + /* Keywords */
 + !^[ 
 \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
   /* Namespace */
   ^[ \t]*(namespace[ \t]+.*)$,
   /* -- */
 
 Here, you are moving keywords down, but in the commit message you say
 that you moved keywords to happen before modifier parsing. Aren't you
 moving keywords *after* something? (Where the modifiers are here is
 not obvious, but that can be attributed to that I don't do C#.)
 

It was a typo because I was sleepy. It was intentional to move them *after*.
Modifier parsing can contain keywords, so just to be sure, I moved the
keywords after modifier parsing, so it uses the keywords as a fallback.
If this is not what should happen, please tell.

Modifiers are prefixes to methods/properties. (the pipe separated lists)

 BTW, I appreciate that you re-arrange keywords alphabetically. Could you
 do that in the commit message, too?
 
 — Hannes

I’ll handle this too, no problem.

Thanks,
Marius

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Marius Ungureanu 
On a side note, I noticed some of the keywords I added shouldn’t be there.

I just realised that simple statements have no reason to be there, but only
block definitions. I’ll reduce the size of this patch on the keywords part.

Thanks,
Marius--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Johannes Sixt 
Am 26.04.2014 11:55, schrieb Marius Ungureanu:
 On 26 Apr 2014, at 10:10, Johannes Sixt j...@kdbg.org wrote:
 Am 26.04.2014 01:25, schrieb Marius Ungureanu:
 diff --git a/userdiff.c b/userdiff.c
 index fad52d6..7612c5d 100644
 --- a/userdiff.c
 +++ b/userdiff.c
 @@ -133,14 +133,14 @@ PATTERNS(cpp,
  |[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*
  |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 -/* Keywords */
 -!^[ 
 \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
  /* Methods and constructors */
 -^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 -/* Properties */
 -^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
 +^[ 
 \t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 +/* Properties, events, delegates */
 +^[ 
 \t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*((delegate|event)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ 
 \t]*$\n
  /* Type definitions */
 -^[ 
 \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +^[ 
 \t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +/* Keywords */
 +!^[ 
 \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
  /* Namespace */
  ^[ \t]*(namespace[ \t]+.*)$,
  /* -- */
 
 ...
 Modifier parsing can contain keywords, so just to be sure, I moved the
 keywords after modifier parsing, so it uses the keywords as a fallback.
 If this is not what should happen, please tell.

For each line, patterns are are scanned in order, and the first match
determines the outcome: If it is a negative pattern (i.e., it begins
with an exclamation mark), the line is not a hunk header; if it is a
positive pattern, the line is a hunk header. If no pattern matches, the
line is not a hunk header, either; it is as if the list were terminated
by a negative catch-all pattern.

Due to these rules, negative patterns in the list are only necessary
when you want to make an exception to a positive pattern in the list,
and then the negative pattern must be listed before the positive pattern.

In the csharp case, I do not see a pattern of which the keyword pattern
would make an exception (neither the old version nor your new version).
Therefore, you could drop the keyword pattern entirely.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Marius Ungureanu 
On 26 Apr 2014, at 20:49, Johannes Sixt j...@kdbg.org wrote:
 Am 26.04.2014 11:55, schrieb Marius Ungureanu:
 On 26 Apr 2014, at 10:10, Johannes Sixt j...@kdbg.org wrote:
 Am 26.04.2014 01:25, schrieb Marius Ungureanu:
 diff --git a/userdiff.c b/userdiff.c
 index fad52d6..7612c5d 100644
 --- a/userdiff.c
 +++ b/userdiff.c
 @@ -133,14 +133,14 @@ PATTERNS(cpp,
 |[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]*
 |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 -   /* Keywords */
 -   !^[ 
 \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
 /* Methods and constructors */
 -   ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 -   /* Properties */
 -   ^[ 
 \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
 +   ^[ 
 \t]*(((abstract|extern|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 +   /* Properties, events, delegates */
 +   ^[ 
 \t]*(((abstract|internal|new|override|private|protected|public|sealed|static|unsafe|virtual)[
  \t]+)*((delegate|event)[ \t]+)*[][@.~_[:alnum:]]+[ 
 \t]+[@._[:alnum:]]+)[ \t]*$\n
 /* Type definitions */
 -   ^[ 
 \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +   ^[ 
 \t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[
  \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n
 +   /* Keywords */
 +   !^[ 
 \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
 /* Namespace */
 ^[ \t]*(namespace[ \t]+.*)$,
 /* -- */
 
 ...
 Modifier parsing can contain keywords, so just to be sure, I moved the
 keywords after modifier parsing, so it uses the keywords as a fallback.
 If this is not what should happen, please tell.
 
 For each line, patterns are are scanned in order, and the first match
 determines the outcome: If it is a negative pattern (i.e., it begins
 with an exclamation mark), the line is not a hunk header; if it is a
 positive pattern, the line is a hunk header. If no pattern matches, the
 line is not a hunk header, either; it is as if the list were terminated
 by a negative catch-all pattern.
 
 Due to these rules, negative patterns in the list are only necessary
 when you want to make an exception to a positive pattern in the list,
 and then the negative pattern must be listed before the positive pattern.
 
 In the csharp case, I do not see a pattern of which the keyword pattern
 would make an exception (neither the old version nor your new version).
 Therefore, you could drop the keyword pattern entirely.
 
 -- Hannes
 

Hey!

I’ll remove them and add as many unit tests I can. I’ve been side tracked
today and I couldn’t get to look at it. I’ll start a new thread with the new
patch as soon as I’m done with it.

Thanks for all the tips until now. You’ve been of great help.

Marius--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Updated C# userdiff patterns.

2014-04-26 Thread Johannes Sixt 
Am 26.04.2014 20:33, schrieb Marius Ungureanu:
 ... add as many unit tests I can.

Great! Keep in mind that quantity is secondary. Quality counts.

 I’ll start a new thread with the new
 patch as soon as I’m done with it.

If possible, do not start a new thread, but post your new patch as a
reply in this thread.

Thanks,
-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html