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

Add more scopes to the tree-sitter grammar #620

Merged
merged 7 commits into from Nov 8, 2018

Conversation

Projects
None yet
3 participants
@Ben3eeE
Member

Ben3eeE commented Nov 1, 2018

Fixes #619
Fixes #617
Fixes #616
Fixes #566

/cc: @maxbrunsfeld

@Ben3eeE Ben3eeE force-pushed the b3-more-scopes branch from e96568a to e04d8bc Nov 1, 2018

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Nov 1, 2018

@chbk This fixes the issues you reported earlier except default is staying scoped as keyword.control

@chbk

This comment has been minimized.

chbk commented Nov 1, 2018

Great! I tried out your PR and it fixes most issues except:

  • this is still source.js
  • super and super() are scoped differently
@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Nov 1, 2018

Yeah super() is a function call and is expected to be scoped differently, while super is a language variable and scoped as such,.

We scope:
call_expression > super as support.function
and super as variable.language

this is because I messed up when staging. Thanks for noticing and testing 👍 💯 Fixed in the latest commit.

@chbk

This comment has been minimized.

chbk commented Nov 1, 2018

Alright, super() being scoped differently makes sense. Would it be possible be more specific then, with support.function.super for example? It would be nice to be able to highlight super the same way in both cases, even if they are syntactically different. After all they refer to the same parent class, super() calling its constructor and super accessing any of its methods.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Nov 1, 2018

I'm ok with making it support.function.super. It still looks the same with one-dark-syntax and one-light-syntax.

@maxbrunsfeld I think this is ready for review now 😅

Ben3eeE and others added some commits Nov 3, 2018

@maxbrunsfeld maxbrunsfeld force-pushed the b3-more-scopes branch from 6e47b33 to cab77b6 Nov 8, 2018

@maxbrunsfeld maxbrunsfeld merged commit f5706d5 into master Nov 8, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

maxbrunsfeld added a commit to atom/language-typescript that referenced this pull request Nov 8, 2018

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Nov 9, 2018

@chbk We removed some scope mappings for language.variable before merging this to get the other changes out in Atom 1.32.2. language.variable is awaiting review of atom/atom#18383 before we change them.

@maxbrunsfeld maxbrunsfeld deleted the b3-more-scopes branch Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment