-
Notifications
You must be signed in to change notification settings - Fork 1
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
Upgrade embedded Python to syntax v2 #13
Conversation
This commit fixes a compatibility error with ST 4148, which ships with a sublime-syntax v2 Python. Note: This fix breaks compatibility with older ST builds. Thus a new release branch is required.
Thanks for the PR, and for alerting me to this change. I have tried duplicating the Python file (keeping the current Also, there's this in the embedding file. I presume this still hasn't landed? Is there any problem with leaving the code uncommented? string-prototype:
# Note: not yet supported by ST 4143/4146 (it's in preparation)
- meta_prepend: true
- include: scope:source.just#recipe-content-string-interpolations |
Having 2 python syntaxes in the package at the same time doesn't make much sense as only one of them will ever be used. The strategy how to go on depends on whether future changes/improvements of Just syntax should still be compatible with ST4075 - 4147. strategy 1 - linearYou could restrict current release to 4075 - 4147 and leave it behind untouched. As ST up to 4147 still ships the v1 python syntax, everything is fine as it is - no vendoring python syntax needed. A next release with this PR merged would need to be restricted to 4148+ (e.g. by using tag prefixes such as This way Users with older builds (before 4148) wouldn't receive further updates to Just syntax. strategy 2 - branching offYou could create a st4148 branch with this PR merged. Primary changes to Just would go on in master. With each release master can be merged into st4148 branch. In master branch a tag with 4075-1.x.x is created and after merging a 4148-1.x.x tag is created in st4148 branch. This way you could still maintain Just for older ST builds while releasing 2 different variants depending on ST build. I do it this way with https://github.com/SublimeText/Astro and various other syntax packages. In neither case vendoring is needed. Vendoring current v1 python syntax would be required only, if you don't want to re-use v2 syntax of 4148+, but I wouldn't recommend that.
It has landed in ST4148+, but not for those who are still using latest stable release (ST4143). This context being present or not is not of any problem. It wouldn't just have any effect if base syntax doesn't know about it. If you vendor v1 python it doesn't make sense to keep it, but I don't have enough context to give any advice about it. (Either vendor v1 python and forget this PR, or use this PR with one of strategy 1 or 2). |
The more I think about this, the more I'm leaning toward just leaving both versions of the embedded Python syntax in place permanently. Based on your suggestions, it looks like if I'm generating tagged builds with only the single correct embedded Python syntax version, then my options are:
You're right that only one embedded syntax will ever be used, but the way I see it, that's actually perfect. It's analagous to a universal binary where all the pieces are available for 2+ architectures. In this case, it's just two mutually-exclusive versions of the syntax. ST will figure out which one it needs based on what it supports, and I don't have to fork anything or do any version/tag work at all. I even avoid the package control PR. And the only cost is a single, tiny file kept around to support older versions. My limited testing seems to show that this works, but I want to make sure I'm not misunderstanding something fundamental here. Other than strict code cleanliness, is there a reason not to include both embedded syntax versions? Thanks very much for your help on this! |
I wouldn't recommend adding both python versions. In best case the v2 syntax is just ignored and Just only uses vendored v1 syntax on all builts. So the v2 would be of no use and wouldn't need to be present. In worst case both being present, using same main scope, could cause conflicts or maybe other unexpected weird effects. If you don't want to create tagged releases, just vendor v1 python and go with it and don't benefit from any future improvements of core python syntax. |
Hmm, interesting. When I add a change to only the embedded v1 syntax, but leave both syntax files in place, the change is only shown in 4143. Doesn't this mean that 4148 is ignoring the v1 syntax and only using the v2 syntax? I don't mean to argue with you! Just trying to understand what I'm seeing. |
It "works" because both syntaxes are inherited from same base syntax while only one compiles successfully. Nevertheless console shows an error message for the failed one in both builds 4143 and 4148.
Exactly this message was, what triggered this PR, while doing research for (sublimehq/Packages#3692). So in this very special case, having both syntaxes in place wouldn't cause any harm, beyound the error message and ST failing to compile a syntax, but in general I still wouldn't recommend it... a) to avoid that misleading error message, |
I have created the I am now going to create the |
I've completed this. Thank you for alerting me to the necessary changes. |
One test failure to resolve (#16) before this can be closed and the 4148+ build released. |
This PR fixes a compatibility error with ST 4148, which ships with a sublime-syntax v2 Python.
To create a new release branch, I'd recommend to prefix your tags with the least required ST build number.
Example:
A corresponding change in package_control_channel would be required.