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

Improve with_prototype to better match Sublime. Fixes #101 #103

Merged
merged 2 commits into from
Sep 5, 2017

Conversation

trishume
Copy link
Owner

@trishume trishume commented Sep 5, 2017

I did a bunch of testing and figured on the details of Sublime's with_prototype behaviour around cases like described in #101.

This PR implements a fix that:

  1. Corrects the order I was iterating with_prototypes in the stack. It is now bottom to top like Sublime.
  2. Doesn't use with_prototype contexts from stack frames lower than the last frame pushed by a with_prototype rule.

This reduces the number of failing syntax tests to 162 for ASP, 1 for LaTeX, and everything else passes. Previously ASP had 202 failing tests, fixing the order only got that down to 183. PHP also used to have 40 failing tests, although that jumped to 104 when I fixed the order, but that was just further exposing the other bug that when fixed brought it down to 0 failures. So this fixes #101 but not yet #59.

TODO

cc @keith-hall

When I first wrote that code I assumed that prototypes were used from top to
bottom but I tested it and it's actually bottom to top.

This reduces the number of failing ASP tests but increases the number of
failing PHP tests. I expect the new PHP failures might just be exposing
another bug.
@trishume trishume merged commit 0b7716f into master Sep 5, 2017
@trishume trishume deleted the improve-with-proto branch September 5, 2017 19:51
@trishume trishume mentioned this pull request Sep 5, 2017
4 tasks
@robinst
Copy link
Collaborator

robinst commented Sep 6, 2017

Nice! Is there some description of the sublime-syntax format that we can expand to add this information? Would be good to have a better description of how they work, for future us but also other implementers.

@trishume
Copy link
Owner Author

trishume commented Sep 6, 2017

@robinst There's https://www.sublimetext.com/docs/3/syntax.html, but it's written for syntax-writers, not implementers. So it heavily underspecifies a lot of things. It's not easy to submit updates, and if it was it wouldn't be the right place.

Maybe a .md file in the Sublime packages repo, or on the wiki of that repo, would be the right place. Or even in syntect's repo or wiki. I guess the source of syntect itself kind of documents every known behaviour, even if it is spread out.

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.

with_prototype syntax test failures
2 participants