Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kotlin: Highlight the type of a property #1322

Merged
merged 1 commit into from
Sep 7, 2019
Merged

Kotlin: Highlight the type of a property #1322

merged 1 commit into from
Sep 7, 2019

Conversation

lordcodes
Copy link
Contributor

@lordcodes lordcodes commented Sep 7, 2019

Highlight the type for a property. E.g. for below highlights 'Boolean' as Name::Class.

val live: Boolean

Fix nested generics. E.g. for below now correctly highlights both instances of 'Array' as Name::Class and Boolean as Name::Class.

private val live: Array<Array<Boolean>>

@pyrmont

Fix nested generics, i.e. Array<Array<Boolean>>
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Sep 7, 2019
@pyrmont pyrmont self-assigned this Sep 7, 2019
@pyrmont
Copy link
Contributor

pyrmont commented Sep 7, 2019

@lordcodes Apologies for what might be a dense question but is there any connection between highlighting the type of the property and allowing nested generics? Those seem unrelated and better submitted as separate PRs.

@pyrmont
Copy link
Contributor

pyrmont commented Sep 7, 2019

Oh, and separately, could you add an example to the visual sample to show this working (perhaps simply the code you included in the OP)?

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Sep 7, 2019
@lordcodes
Copy link
Contributor Author

lordcodes commented Sep 7, 2019

@lordcodes Apologies for what might be a dense question but is there any connection between highlighting the type of the property and allowing nested generics? Those seem unrelated and better submitted as separate PRs.

They can be done as separate PRs, as they technically involved separate code changes. However, they are a related concept. The fix for nested generics was within a case that was called from the property rule that was fixed for the other part of the PR. Both are in the property type, so in my opinion are related and best just fixed together.

The nested generic wasn't an issue previously, as the property type wasn't highlighted at all. I.e. the first change meant that change was also needed.

@lordcodes
Copy link
Contributor Author

Oh, and separately, could you add an example to the visual sample to show this working (perhaps simply the code you included in the OP)?

Both parts can be seen in the visual sample, i.e. they weren't highlighted before and now they are.

@pyrmont
Copy link
Contributor

pyrmont commented Sep 7, 2019

@lordcodes Roger that on both points. Will just check out and do a sanity check but should be good to merge.

@lordcodes
Copy link
Contributor Author

@pyrmont Perfect, thanks!

@pyrmont pyrmont merged commit cdb3592 into rouge-ruby:master Sep 7, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label Sep 7, 2019
@lordcodes lordcodes deleted the kotlin-property-types branch September 7, 2019 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants