-
Notifications
You must be signed in to change notification settings - Fork 105
New Cargo build system. #165
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
Conversation
I agree about the "rustc" build variants, they're not really needed now we have Cargo Script. So im fine with that |
Looks like there could be some big, potentially breaking changes in this (especially if rust build is being removed) So marking as a potential for 2.0.0 for now |
This includes a dependency on shellenv to capture the user's environment from their login shell.
This fixes some problems running tests on Windows.
This was causing random breakage (particularly on Windows).
This includes some significant changes to how the current package is selected. It should work a little better if you have multiple packages.
…hed. This fixes issues with "cargo run" if your program outputs { at the start of a line.
The previous fix caused issues with Clippy.
Add commands to configure extra arguments and environment variables.
Also document the `default_path` option.
@ehuss im back now, how did you want this reviewed? |
Good question. I don't have anything else left that I am planning to do, so it should be ready to go. I think primarily testing it out and seeing if you have any obvious problems. I've tested on Windows, Mac, and Linux, but it always helps to try on someone else's setup. See if the usage and documentation make sense and is not too confusing. I have not yet switched the default on-save syntax check to |
Not ignoring this just been pretty busy, and maybe this weekend too. @ehuss are you on the Rust irc server? May be good to catch you in real time when going through this, no worries if not |
Been taking a look at this today, first off great work, I like the documentation you've added around Cargo build and i like the addition of Automatic. Everything still seems to work fine from my end and nothing is broken. When i click "Configure Cargo Build" then "Set Target", it sends me to a build menu, like "Automatic", "Bench", "Check" is it supposed to do this? When attempting to add an environment variable it opens a new window (with RUST_BACKTRACE: 1 already in it commented out) if i type nothing and try to close the windows sublime text crashes, does this happen for you? (ST3 3126) Windows 10
@dten are you able to look at this too? |
The "Set Target" sets the default target per build variant. So if you want to run just 1 test file, you choose "Test > Test: test_name". Then when you run the "Test" variant, it always uses that target. I felt like it didn't make sense to have a global default target since targets are often specific to a particular variant. I think it would be nice if it would only show targets for the currently active build variant, but AFAIK it is not possible to ask Sublime which is currently active. Yes, you found a bug with the environment variables. I just pushed a fix, thanks! |
@ehuss ok i see
If Cargo is using Target to identify the same thing we should stick with it |
The remaining unchecked items are mostly nice-to-haves or random thoughts. I'll go through and clear out what's left and add a few more tests. I guess the only issue I was really uncertain about is: Should Clippy and Bench default to running on nightly? Clippy should work correctly without that if you have your PATH/LD_LIBRARY_PATH set correctly, but I assume most people don't do that (it would be nice if that was fixed in rustup). Also, if clippy is later distributed as a rustup component, it won't be necessary. I have no idea what the future holds for "bench". |
Just wondering how we feel about merging this? |
I'm ready for it to be merged and to address any issues that crop up. |
Hi guys, I've been a way lately but should have some time next week to look at this, and hopefully we can get a 2.0.0 release going with this in |
Looks good to me, merged |
* New Cargo build system. * Fix wrapping of commands in GitHub flavored markdown. * Minor fixes to unittests for Rust 1.16. * Add support for "cargo check". * Support clippy for on-save checking. * Fix on-save syntax check running clippy multiple times. * Add ability to configure cargo build features. * Support setting environment variables. This includes a dependency on shellenv to capture the user's environment from their login shell. * Normalize paths when dealing with settings. This fixes some problems running tests on Windows. * Disable on-load message display for tests. This was causing random breakage (particularly on Windows). * Fix race condition in tests manifested on linux. * Add support for custom build variants. This includes some significant changes to how the current package is selected. It should work a little better if you have multiple packages. * Add support for a default path/package. * Add "Automatic" build variant. * Stop looking for JSON output during Cargo build once compile is finished. This fixes issues with "cargo run" if your program outputs { at the start of a line. * Better fix for checking when we should stop looking for JSON output. The previous fix caused issues with Clippy. * Remove debug print left behind. * Minor updates for rust 1.17. * Change message tests to check the region of the message. * Fix error highlighting for nested macros. * Fix clearing of error regions. * Add rust_phantom_style and rust_region_style config settings. Fixes rust-lang#182. * Honor show_errors_inline config setting for Cargo build. * Fix links in messages that are surrounded by angled brackets. * Consolidate all Cargo config commands into a single command. Add commands to configure extra arguments and environment variables. * Document the new build configure command. Also document the `default_path` option. * On-save syntax checking now uses configuration settings from the build system. * Finish documenting all settings, and include link to new build docs. * Fix Cargo environment variable config command when selecting "all build commands". * Only print warning about sublime-project when saving setings. * Fix target detection for cdylib. Adapted from rust-lang#186, thanks @smbolton. * Update tests for new messages in rust 1.18.
I wanted to share a WIP of a new, enhanced build system for Cargo. I was wondering:
The main improvements are:
This also closes a few issues: #182, #170, #162, #154, #146, #109
It should be fully functional in its current state, but I still have a long todo/improvement list. Here's a copy in case you are curious:
Test with a variety of message types (to ensure is_main is good)
Test with already open files and files that need to be opened.
[ ] Run with multiple executables displays this error:Deferrederror:
cargo run
requires that a project only have one executable; use the--bin
option to specify which one to runShould we detect this, and bring a popup to select a target?
[ ] I think the build-language could be improved (coloring file filenames, numbers, etc.)Deferred[ ] Something similar to Anaconda Show Errors (popup with list of errors)Deferred[ ] Other cargo test options?: --doc --no-run --no-fail-fastDeferredInternally Cargo uses various terms (CompilerFilter, Kind, Target)
One last thing, this replaces the "rustc" build variants. I'm not sure what the use case is for those, and whether or not "cargo script" is a sufficient replacement.