grammar fix #11: method signatures with generic parameters

atom / language-csharp

C# language support for Atom

Other

62 stars 52 forks source link

grammar fix #11: method signatures with generic parameters #51

Closed andycmaj closed 7 years ago

andycmaj commented 8 years ago

Before

Notice how method names with Generic parameters aren't parsed as entity.name.function.cs. You can tell because Register<TEvent> has no style applied. Also, Generic parameters aren't recognized in method return-types.

After

Method names with Generic parameters are recognized as entity.name.function.cs, and the Generic params themselves are recognized. Generic params in return-types are also recognized.

Known issue

Can't seem to figure out how to get generic params to be recognized after the first param. Tried to copy what #parameters pattern does, but doesn't seem to work here. Any help would be appreciated, but this is still an improvement even without this sub-fix.

andycmaj commented 8 years ago

Would like to add more specs for this... figuring out how to do that... jasmine?

winstliu commented 8 years ago

@andycmaj You might want to take a look at my language-java PR which also adds Generics highlighting: atom/language-java#22.

andycmaj commented 8 years ago

@50Wliu, thanks... any advice on what I should do differently here? Don't want to go re-writing the entire csharp grammar, as it seems like most things work fine right now...

some specific feedback on my change would be good.

thanks. excited to start contributing to atom stuff

damieng commented 7 years ago

Big rewrite of the grammar just got merged that takes care of generic method parameters and their constraints.

Also has same issue of return-types not highlighting multiple args properly however. Doing that is going to require rewriting the method detection somewhat significantly and is non-trivial.

damieng commented 7 years ago

Sorry your PR didn't get merged in - that reworking was quite complex even without bringing additional PRs into scope :(