atom / language-csharp

C# language support for Atom
Other
62 stars 53 forks source link

Fix issue with strings in method calls #36

Closed pchaigno closed 9 years ago

pchaigno commented 9 years ago

This pull request fixes an issue reported in #28:

class F {
  a = C("1.1 10\n");
  b = 5;
}

See the fix in Ligthshow.

EDIT: Also fixes #32.

50Wliu commented 9 years ago

So if I'm understanding this correctly, was the issue that the entire a = C part was getting marked as meta.method.source.cs?

pchaigno commented 9 years ago

So if I'm understanding this correctly, was the issue that the entire a = C part was getting marked as meta.method.source.cs?

Yep, exactly!

50Wliu commented 9 years ago

I'm a bit uncomfortable with the notion of checking that a = isn't there. Would it be possible to instead just make sure there's only spaces between the letter and the parentheses, eg (?!new)[\\w<]\\s*\\(? I'm not seeing the point of requiring a space and then another non-equals sign (\\s+[^=]+).

pchaigno commented 9 years ago

It is the regex for the method definitions. We need the = to make sure it's not an assignment. We need the space between the two groups to make sure it's not just a method call. We need to match the whole function definition and not just the parenthesis because afterwards, sub-regex captures the return type, modifiers and the name of the method.

To fix #32, we also need to make sure there isn't any // or /*.

In short, we need to match:

void func()
dictionary<int, string> func()
void func(test = default_value)

but not:

a = func(1)
call_to_method()
byte end; //(
byte end; /*(*/
call_to_method  ()
call_to_method ()

I think we could "simplify" the whole regex by grouping the things we don't want at the beginning with a negative lookahead. My last commit implements that.

50Wliu commented 9 years ago

:articulated_lorry: