Re-upstream syntax to SublimePackages

[dten]

In a recent issue @wbond asked if we might take some time to contribute to the default Rust syntax in Sublime again.
sublimehq/Packages#1669 (comment)

With recent push to fix things here we should definitely try contribute those back. Some changes have been made to the Sublime Packages so I'm not sure it's as easy as it might look but both have tests so it's probably a case of merge the tests and see what fails with this syntax.

[jasonwilliams]

Yeah last time they just did it themselves, so I'm not particularly sure what they need from us?
They don't want any of the JS stuff, so they could just take the syntax definition and force merge it on their side?
@wbond?

[wbond]

I'm mostly interested in bug reports, to be honest. I know you guys are attempting to do different things here in terms of custom scope names and extra features, but since Rust Enhanced is based on the default packages, when you find a bug, if you could report it, then we can keep the default Rust package in good shape.

This is probably especially true if/when Rust adds new syntax, etc. Providing a snippet of new syntax would allow us to make sure we are tracking the latest Rust enhancements. We really rely on having language experts provide feedback, and then the core community of syntax maintainers can combine that with all of their knowledge of Sublime Text and .sublime-syntax.

[ehuss]

(Caveat: I'm not familiar with the original reasons the syntax definitions split in the first place.)

@wbond I'm not aware of any custom scope names. AFAIK almost all the scope names are best-effort trying to match the scope naming guidelines. I'm also not sure what you mean by extra features, at least I'm not aware of anything particularly special in the syntax definition. There are probably around 50+ issues in the stock Rust definition at this point. It would be quite a bit of work just to enumerate all of them.

If someone wants to, they can look at the most recent issues here, but I only started tagging issues within the past few weeks. I've also expanded the test suite quite a bit here.

It's never been clear to me why the stock Rust definition needs to be so different. In its current state, it is pretty far behind.

[wbond]

@ehuss The only reason it would be far behind would be a lack of bug reporting and/or not sending PRs.

Back in 2016 I did a pretty significant rewrite from scratch since existing syntaxes didn't fit in with the default packages. The issues were generally poor coverage of the syntax, poor scope names, no tests and usage of the slow regex engine.

In late 2016 the syntax was forked from the default syntax - #108.

The scope naming that I was thinking of was https://github.com/rust-lang/rust-enhanced/pull/116/files. Things like entity.name.type, which is against the scope naming guidelines at https://www.sublimetext.com/docs/3/scope_naming.html#entity. It appears that eventually was closed. I haven't checked the scopes recently.

Unfortunately after the fork, the Rust community seemed to more or less ignore the default syntax - see https://github.com/sublimehq/Packages/issues?utf8=%E2%9C%93&q=is%3Aissue+Rust. Only a handful of bug reports.

My comment that started this was that I feel like it is a shame that a fork happened which resulted in the default syntax seeming to have been ignored by the Rust community. Ideally the default Rust syntax would get bug reports (and even pull requests) from frequent users so that all Sublime Text users can get the best Rust syntax highlighting available.

I'd love to see all of the bug fixes make their way back into the default syntax. I think RustEnhanced is the right place for linting, etc. But for the syntax, that would ideally live in one place, rather than having two syntaxes diverging and having to be reconciled.

From a practical perspective, we'd probably need to start with using tests from both (unless you've been pulling in the handful of bug fixes that have landed in the default syntax), and then reviewing the current state of the RustEnhanced syntax and seeing where tweaks may need to be made for the default syntax. Possible things to review include:

• scope names
• ensuring the oniguruma engine is never invoked due to look behinds or back references

[dten]

What is the release cadence of the main Packages syntaxes? I was under the impression they went out with Sublime itself and thus syntax fixes were not able to be responsive.
I don't think forking is best for everyone.

[wbond]

Yes, pretty much every dev build gets all of the tweaks that have made it into the repo since the last build.

[urschrei]

Given the frequency with which language and syntax changes are currently landing in stable Rust (every six weeks), and given the (relative to Rust's) far lower frequency of stable Sublime releases (two this year, AFAIK), the default package is going to lag significantly for people who are on stable Sublime. What could we do about that?

[ehuss]

I feel the release frequency for Sublime is too low for Rust for the syntax to only live in the default packages. If we can sync them, I'd be fine with sending changes upstream, though.

I can try to do a review of where the (non-broken) scopes are different and try to figure out how to resolve that. It looks like one oniguruma pattern slipped through, we should be able to fix that (and maybe add a CI check for that).

If the only scope differences are due to fixes or improvements, would it be possible to merge the Rust Enhanced syntax entirely? Trying to break up the changes into individual bits would be too much work for me (like I mentioned, there's a lot, and I have more on the way).

[wbond]

I feel the release frequency for Sublime is too low for Rust for the syntax to only live in the default packages. If we can sync them, I'd be fine with sending changes upstream, though.

Well, the current situation is arguably far worse. Most users of Sublime won't have RustEnhanced installed, and instead will have an even older version of the Rust syntax without enhancements for new syntax, and without bug fixes.

If we can sync them, I'd be fine with sending changes upstream, though.

I don't see why RustEnhanced couldn't periodically grab Rust.sublime-syntax from master and merge it in. This way the entire community of .sublime-syntax experts at https://github.com/sublimehq/Packages can contribute their knowledge, and the Rust community can also participate.

It looks like one oniguruma pattern slipped through, we should be able to fix that (and maybe add a CI check for that).

You you using the syntax_tests binary for CI tests currently?

If the only scope differences are due to fixes or improvements, would it be possible to merge the Rust Enhanced syntax entirely?

I would say start with creating a PR that adds the current version of RustEnhanced with all of your new tests, but without removing or changing any tests from sublimehq/Packages and we can move from there. Ideally there shouldn't be a reason that any MR can't pass the current tests, unless they are faulty, which we should be able to identify.

I imagine some of the regular contributors will have suggestions or requests for the MR, but I see know reason why we shouldn't be able to work through them. It doesn't seem to me that we need the full commit history.

[jasonwilliams]

@wbond maybe I’m misisng the point here, but why do we need 2 separate .sublime-syntax files with separate effort going on for each one, when we’re all trying to achieve the same objective.
I wouldn’t say the “rust community have ignored the default syntax” it’s more that they’re focused here in this repo. We clearly need a single source of truth.

I agree with @ehuss we should just try to fix those issues here and you can merge from upstream.
I’m not sure how well bug reporting willl go, as it may be confusing for users who already expect syntax fixes to happen here, where we can be more responsive and get releases out in a more timely manner

[wbond]

@jasonwilliams I see no reason why there should be two different syntaxes, which is kind of my point. Why don't we continue to collaborate at the sublimehq/Packages repo where this started? We aren't going to drop the Rust package, and it would be really nice if the Rust community contributed back to the syntax that RustEnhanced was forked from.

[ehuss]

I don't see why RustEnhanced couldn't periodically grab Rust.sublime-syntax from master and merge it in.

It's a little discouraging to see months of PRs backed up in sublimehq. If it takes more than a couple weeks to get changes through, I'd rather not go that direction. As I mentioned, I'm fine with periodically sending PRs upstream.

You you using the syntax_tests binary for CI tests currently?

No, we are using the UnitTesting package which runs sublime_api.run_syntax_test. Is there much of a difference between them?

I would say start with creating a PR that adds the current version of RustEnhanced with all of your new tests, but without removing or changing any tests from sublimehq/Packages and we can move from there.

Once most of the open issues are addressed, I can look at doing this.

[jasonwilliams]

@wbond I tried to catch you on discord. We should have a chat about it.

Some thoughts:
Just to clear up a bit of history; Although the new syntax file was forked from default packages, this repo has been around before rust syntax started being baked into Sublime 3, you could argue that collaboration on rust syntax for sublime began here. Before your very good rewrite we were using the old tml version.

I still don’t know the release cycle for stable users of sublime, it would make sense for the canonical version to live here (this is the official rust organisation after all) and the default packages repo can merge in changes when they feel fit. This allows rust users to stay up to date, whilst sublime users are only slightly behind if they don’t want to install the package. It would be a huge disservice to rust developers to go from weekly releases to twice a year.

Bringing syntax changes inline with sublime releases can be a bad idea, one of the reasons sublime has survived for so long is because packages and syntax can continue to be active when core releases grind to a halt, which has happened in the past, who knows if it will happen again. So there is that level of risk which has already been pointed out.

With all that said, we would appreciate input on conventions, and should try to work with the syntax experts on changes and PRs. Maybe this is something we should discuss.