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

Tokenize formal function parameters in tree-sitter grammar #297

Merged
merged 1 commit into from
Apr 10, 2019

Conversation

caleb531
Copy link
Contributor

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

❤️

@nathansobo nathansobo merged commit 283fba7 into atom:master Apr 10, 2019
@nathansobo nathansobo self-assigned this Apr 10, 2019
@caleb531
Copy link
Contributor Author

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

@winstliu
Copy link
Contributor

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

@Arcanemagus
Copy link

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

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

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

Successfully merging this pull request may close these issues.

4 participants