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

Update Packages and add support for clear_scopes #27

Merged
merged 6 commits into from
Feb 5, 2017
Merged

Conversation

trishume
Copy link
Owner

@trishume trishume commented Dec 22, 2016

  • Update the Sublime Packages submodule to the latest version
  • Add clear_scopes to the SyntaxDefinition type
  • Fix test failures due to updated syntaxes
  • Implement the new clear_scopes setting in the parser
  • Benchmark and ensure that performance hasn't regressed.

I'm currently waiting on a reply to my comment on sublimehq/Packages#466 to see if I understand clear_scopes before implementing it. I'm planning on storing a Vec<Scope> in StateLevel of scopes to push back on to the stack when that level is popped.

fixes #26

cc @robinst

@trishume trishume changed the title [WIP] Update Packages and add support for clear_scopes Update Packages and add support for clear_scopes Feb 4, 2017
@trishume
Copy link
Owner Author

trishume commented Feb 4, 2017

So I ran some benchmarks and it seems that these changes come with a 4% performance regression with the same version of the JS sublime-syntax, and an 8% regression comparing this branch to master including the newer JS sublime-syntax. The new syntax has more features and gives better results so I think the 4% is a better number, and I think that's acceptable. I'll also try running the other benchmarks.

@trishume trishume merged commit b7ec308 into master Feb 5, 2017
@trishume trishume deleted the clear-scopes branch February 5, 2017 00:21
@robinst
Copy link
Collaborator

robinst commented Feb 6, 2017

Would you mind cutting a release with these changes included (probably 1.1.0)? Switching over the regex engine might take a while :).

@trishume
Copy link
Owner Author

trishume commented Feb 6, 2017

@robinst done.

@robinst
Copy link
Collaborator

robinst commented Feb 7, 2017

Thanks, works nicely!

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.

Update included dump of Sublime text syntax definitions?
2 participants