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

Upstream official Rust package #178

Closed
Manishearth opened this Issue Feb 16, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@Manishearth

Manishearth commented Feb 16, 2016

Currently Rust maintains a Sublime package here which handles syntax/snippets/etc. It does not handle autocompletion, though there are packages for that.

It would be nice if it was bundled by default. Opening this bug to see if there are any objections to that before opening a PR 😄

I assume it's a straightforward matter of copying the Rust package into this folder, but of course I'll test it out first.

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 16, 2016

(Also, in case this is accepted, how would upgrading work for people who have installed the Package Control Rust package and upgrade to a sublime containing the package by default? will the package be silently overridden, or will it cause issues?)

Manishearth commented Feb 16, 2016

(Also, in case this is accepted, how would upgrading work for people who have installed the Package Control Rust package and upgrade to a sublime containing the package by default? will the package be silently overridden, or will it cause issues?)

@wbond

This comment has been minimized.

Show comment
Hide comment
@wbond

wbond Feb 16, 2016

Collaborator

Package Control installs packages as a .sublime-package file into the Installed Packages/folder in Sublime Text 3. (That is, unless the package has a .no-sublime-package file in the root.)

If there is a .sublime-package file in the Installed Packages/ folder with the same name as the default packages shipped with Sublime Text, it will ignore the default package completely, and only use the one from Installed Packages/.

All of that is to say, users who have installed Rust.sublime-package from Package Control will continue to get updates that way. If we were to include Rust as a default package, it would not interfere with users who already have it installed.

Collaborator

wbond commented Feb 16, 2016

Package Control installs packages as a .sublime-package file into the Installed Packages/folder in Sublime Text 3. (That is, unless the package has a .no-sublime-package file in the root.)

If there is a .sublime-package file in the Installed Packages/ folder with the same name as the default packages shipped with Sublime Text, it will ignore the default package completely, and only use the one from Installed Packages/.

All of that is to say, users who have installed Rust.sublime-package from Package Control will continue to get updates that way. If we were to include Rust as a default package, it would not interfere with users who already have it installed.

@jrappen

This comment has been minimized.

Show comment
Hide comment
@jrappen

jrappen Feb 25, 2016

Contributor

If (and only if) the @rust-lang group would then stop maintaining their own package and start updating this one though, you'd be overwriting a possibly newer file here with an older one there. Not implying that would happen, simply stating what could possibly happen if that were the case.

Contributor

jrappen commented Feb 25, 2016

If (and only if) the @rust-lang group would then stop maintaining their own package and start updating this one though, you'd be overwriting a possibly newer file here with an older one there. Not implying that would happen, simply stating what could possibly happen if that were the case.

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 26, 2016

We would continue to maintain if necessary (perhaps just making it a simple copy), and when the sublime version with Rust is released, release a new version of the package control package with release notes mentioning that it's installed by default.

Manishearth commented Feb 26, 2016

We would continue to maintain if necessary (perhaps just making it a simple copy), and when the sublime version with Rust is released, release a new version of the package control package with release notes mentioning that it's installed by default.

@wbond

This comment has been minimized.

Show comment
Hide comment
@wbond

wbond Feb 26, 2016

Collaborator

@Manishearth I am planning on converting it to the new .sublime-syntax at some point in the near future so we can get it into the default packages. Once we have it in a beta release, we can change the version on Package Control to be for ST2 only.

Collaborator

wbond commented Feb 26, 2016

@Manishearth I am planning on converting it to the new .sublime-syntax at some point in the near future so we can get it into the default packages. Once we have it in a beta release, we can change the version on Package Control to be for ST2 only.

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 26, 2016

it as in the Rust package? sounds cool! I thought tmLanguage files were okay in default-packages, though?

Manishearth commented Feb 26, 2016

it as in the Rust package? sounds cool! I thought tmLanguage files were okay in default-packages, though?

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 26, 2016

oh, looks like they aren't. Thanks for offering to convert it; I have no idea how to do that 😄

(though it probably isn't too hard)

Manishearth commented Feb 26, 2016

oh, looks like they aren't. Thanks for offering to convert it; I have no idea how to do that 😄

(though it probably isn't too hard)

@wbond

This comment has been minimized.

Show comment
Hide comment
@wbond

wbond Feb 26, 2016

Collaborator

@Manishearth Yes, I spoke with Jon and we would both like to get Rust included in the default packages.

There is an automated script to convert it to the new syntax. At that point I'll review the current definition and look for any features (such as lookbehinds) that aren't compatible with the newer regex engine and find an alternative way to implement them. This helps improve performance – not only the file load time, but also the speed of the indexer since it tokenizes all the files it finds.

To give you a sense of the performance difference, recently I rewrote the JavaScript syntax and achieved about a 400% improvement in speed by avoiding the few regex features not available in the new engine.

The only other thing I could use from the Rust community is some files that exhibit various syntax, especially edge cases. I'll then write a test file to ensure that we don't have any regressions from future pull requests.

Collaborator

wbond commented Feb 26, 2016

@Manishearth Yes, I spoke with Jon and we would both like to get Rust included in the default packages.

There is an automated script to convert it to the new syntax. At that point I'll review the current definition and look for any features (such as lookbehinds) that aren't compatible with the newer regex engine and find an alternative way to implement them. This helps improve performance – not only the file load time, but also the speed of the indexer since it tokenizes all the files it finds.

To give you a sense of the performance difference, recently I rewrote the JavaScript syntax and achieved about a 400% improvement in speed by avoiding the few regex features not available in the new engine.

The only other thing I could use from the Rust community is some files that exhibit various syntax, especially edge cases. I'll then write a test file to ensure that we don't have any regressions from future pull requests.

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Feb 26, 2016

Ooh, nice.

some files that exhibit various syntax

https://github.com/rust-lang/rust/tree/master/src/test/run-pass contains a ton of testcase files you might want to use to check how the highlighting works.

rustfmt's tests contain more specific syntax examples.

If you'd like more examples of some specific syntax let me know, and I can enumerate how that syntax works.

cc @nrc

Manishearth commented Feb 26, 2016

Ooh, nice.

some files that exhibit various syntax

https://github.com/rust-lang/rust/tree/master/src/test/run-pass contains a ton of testcase files you might want to use to check how the highlighting works.

rustfmt's tests contain more specific syntax examples.

If you'd like more examples of some specific syntax let me know, and I can enumerate how that syntax works.

cc @nrc

@wbond

This comment has been minimized.

Show comment
Hide comment
@wbond

wbond Mar 15, 2016

Collaborator

I've just added an initial version of Rust support with 58cb410.

I started with https://github.com/rust-lang/sublime-rust and did the following:

  • Converted snippets to use tabs so that the user's chosen indentation is always used
  • Converted Rust.tmLanguage to Rust.sublime-syntax
  • Reworked the syntax to follow conventions we've slowly been moving towards. Some of the changes include:
    • meta. scope for sections for larger sections of code
    • Symbol list support for structs, enums, impl, functions, closures and macros
    • Highlighting of various tokens using contextual information
    • Handling of formatting strings with various print/format/write macros
    • Special parsing of macro_rules! to more clearly highlight metavariables
    • Distinction between labels and lifetime indicators
  • Added syntax_test_rust.rs to allow us to fix bugs and make tweaks, hopefully without any major regressions

Our plan is to include this in the next dev build. Anyone with the current Rust package installed via Package Control will not see this package when it is included. They would need to uninstall the version from Package Control to get all of the new features and enhancements.

It may be desirable for the Rust team to make a new release of the package for ST3 that includes an upgrade message indicating future changes will be available along with Sublime Text itself.

It is also possible that the name of the package could be changed from Rust to Rust (Old), or something of the like, to have users automatically start using the new version.

We can discuss these ideas at https://github.com/rust-lang/sublime-rust/issues if you'd like.

Collaborator

wbond commented Mar 15, 2016

I've just added an initial version of Rust support with 58cb410.

I started with https://github.com/rust-lang/sublime-rust and did the following:

  • Converted snippets to use tabs so that the user's chosen indentation is always used
  • Converted Rust.tmLanguage to Rust.sublime-syntax
  • Reworked the syntax to follow conventions we've slowly been moving towards. Some of the changes include:
    • meta. scope for sections for larger sections of code
    • Symbol list support for structs, enums, impl, functions, closures and macros
    • Highlighting of various tokens using contextual information
    • Handling of formatting strings with various print/format/write macros
    • Special parsing of macro_rules! to more clearly highlight metavariables
    • Distinction between labels and lifetime indicators
  • Added syntax_test_rust.rs to allow us to fix bugs and make tweaks, hopefully without any major regressions

Our plan is to include this in the next dev build. Anyone with the current Rust package installed via Package Control will not see this package when it is included. They would need to uninstall the version from Package Control to get all of the new features and enhancements.

It may be desirable for the Rust team to make a new release of the package for ST3 that includes an upgrade message indicating future changes will be available along with Sublime Text itself.

It is also possible that the name of the package could be changed from Rust to Rust (Old), or something of the like, to have users automatically start using the new version.

We can discuss these ideas at https://github.com/rust-lang/sublime-rust/issues if you'd like.

@wbond wbond closed this Mar 15, 2016

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Mar 15, 2016

Thanks for doing this!

We'll coordinate further on rust-lang/rust-enhanced#72

Manishearth commented Mar 15, 2016

Thanks for doing this!

We'll coordinate further on rust-lang/rust-enhanced#72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment