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

Tokenize formal function parameters in tree-sitter grammar #297

Merged
merged 1 commit into from Apr 10, 2019

Conversation

Projects
None yet
4 participants
@caleb531
Copy link
Contributor

commented Apr 6, 2019

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

This PR tokenizes the formal function parameters in the tree-sitter grammar:

Before:
Screen Shot 2019-04-06 at 1 37 55 PM

After:
Screen Shot 2019-04-06 at 1 38 22 PM

Alternate Designs

I don't know if the core Atom team would prefer to use a different scope to tokenize these formal function parameters. However, the variable.parameter.function scope is already used to tokenize keyword arguments in the tree-sitter grammar, so I don't think there's any inconsistency here.

Benefits

Better highlighting for the formal function parameters in Python files matched by the tree-sitter grammar. This matches the highlighting in the existing first-mate grammar.

Possible Drawbacks

None that I can see.

Applicable Issues

N/A

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

❤️

@nathansobo nathansobo merged commit 283fba7 into atom:master Apr 10, 2019

2 checks passed

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

@nathansobo nathansobo self-assigned this Apr 10, 2019

@caleb531

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@nathansobo Wonderful! Any chance these changes will make it into the upcoming Atom v1.37? I only ask because I think it would be fitting complement to my recently-merged #296, which enables the tree-sitter parser more fully for Python files.

@50Wliu

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

@caleb531 We're probably going to want this to go through the entire release cycle to identify any regressions before it reaches stable.

@Arcanemagus

This comment has been minimized.

Copy link

commented Apr 12, 2019

If you absolutely can't wait it should be available in the nightly builds, but note that that channel has zero manual testing on it and things may break at any time as it's built directly from Atom's master branch.

@caleb531

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@50Wliu @Arcanemagus I myself can totally wait—I already have these changes in my init file anyway (thanks to SyntaxScopeMap's addSelector method). I was just thinking about other Atom users who may want the fullest of syntax highlighting sooner, but your reasoning and process make perfect sense, and I respect that.

@50Wliu 50Wliu referenced this pull request Apr 24, 2019

Closed

Atom 1.32 breaks syntax highlighting #281

1 of 1 task complete

@caleb531 caleb531 deleted the caleb531:tree-sitter-function-parameters branch May 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.