Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

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

Closed
wants to merge 3 commits into from

Conversation

andycmaj
Copy link

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.

broken method signatures

### 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.

fixed method signatures

### 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
Copy link
Author

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

'name': 'storage.type.generic.variance.cs'
'2':
'name': 'storage.type.generic.arg.cs'
'end': '(?:(,)|(?=[>]))'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this pattern only seems to catch the first generic param... shoudl work the same way as #parameters, which seems to be applied for each param in a method signature. but it doesn't...

@50Wliu
Copy link
Contributor

50Wliu commented Feb 17, 2016

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

@andycmaj
Copy link
Author

@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
Copy link
Contributor

damieng commented Dec 30, 2016

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
Copy link
Contributor

damieng commented Dec 30, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants