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

Merged
merged 3 commits into from Oct 25, 2018

Conversation

Projects
None yet
3 participants
@Ben3eeE
Member

Ben3eeE commented Oct 25, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

There were a couple of missing scope mappings that made the tree-sitter grammar not match the textmate grammar.

  • Scope block and method parameters to match the textmate grammar
  • Change the class of symbol to match the textmate grammar
  • Scope inherited classes differently from classes to match the textmate grammar

Alternate Designs

N/A

Benefits

Matches the textmate grammar more closely

Verification process

I used the following ruby file to test these new scope mappings

class OrderItem < ApplicationRecord

end

imablock do |param1, param2|
  OrderItem
end

def imablock(param1, param2)
end

def predicate?(date: start_on)
  do_a_thing
end

{key: CONSTANT, key2: 123, key3: false, key4: :symbol}

image

Applicable Issues

Fixes tree-sitter/tree-sitter-ruby#94
Fixes #243
Fixes #242
Fixes #241

Add more scopes
* Scope block and method parameters to match the textmate grammar
* Change the class of symbol to match the textmate grammar
* Scope inherited classes differently from classes to match the textmate grammar

@Ben3eeE Ben3eeE requested a review from maxbrunsfeld Oct 25, 2018

@@ -120,6 +120,7 @@ scopes:
{match: '^[A-Z_]+$', scopes: 'constant'}

This comment has been minimized.

@Ben3eeE

Ben3eeE Oct 25, 2018

Member

This should be changed to variable.constant if we want to match the textmate grammar and different highlighting for constant identifiers and constant literals.

This comment has been minimized.

@maxbrunsfeld

maxbrunsfeld Oct 25, 2018

Contributor

I think let's go ahead and change it.

@maxbrunsfeld

Thanks so much for jumping on this @Ben3eeE! 🚢 at will.

@@ -120,6 +120,7 @@ scopes:
{match: '^[A-Z_]+$', scopes: 'constant'}

This comment has been minimized.

@maxbrunsfeld

maxbrunsfeld Oct 25, 2018

Contributor

I think let's go ahead and change it.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 25, 2018

image

With base16-tomorow-dark. Latest commit.

It seems that variable.constant is not styled in one-dark-syntax so it looks the same as my previous screenshot.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Oct 25, 2018

image

Added #241 to this PR as well so it fixes all open issues. I will merge and make a release PR on atom/atom once it's green.

Image is latest commit one-dark-syntax.

@Ben3eeE Ben3eeE merged commit 34327c5 into master Oct 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Ben3eeE Ben3eeE deleted the b3-add-more-scopes branch Oct 25, 2018

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