rust-lang / rust-mode

Emacs configuration for Rust
Apache License 2.0
1.1k stars 176 forks source link

Add configuration option to locate project without --workspace to only run check on local crate #545

Closed bradneuman closed 1 month ago

bradneuman commented 1 month ago

I'm working in a project that uses a Cargo Workspace and has some large crates. I want to be able to iterate with commands like cargo check on just my crate.

I think one wait to do this would be to add a config to not include --workspace in the cargo locate-project command.

I might take a stab at it, but wanted to post here in case others are interested or there's a better workaround already

bradneuman commented 1 month ago

OK took a crack at it, seems to work for me

psibi commented 1 month ago

Thanks, I saw your PR. I think instead of the approach you have taken here, you should make it accept universal argument to pass custom arguments instead.

That's how we do it in rustic where this workflow is already implemented: https://github.com/emacs-rustic/rustic/blob/17a79c659b0eb304e35705facb114caf61eae2e9/rustic-cargo.el#L703

bradneuman commented 1 month ago

Ah that makes sense, and then just have it default to --workspace?

Although now I might just switch to rustic :-D

psibi commented 1 month ago

Ah that makes sense, and then just have it default to --workspace?

This is the default we have there: https://github.com/emacs-rustic/rustic/blob/17a79c659b0eb304e35705facb114caf61eae2e9/rustic-cargo.el#L90

But yeah, --workspace also seems a reasonable default.

Although now I might just switch to rustic :-D

Cool, note that the installation instruction might need some updates etc. Feel free to send PR's. The fork is not officially announced yet. For more background, here is the information: https://psibi.in/posts/rustic.html

bradneuman commented 1 month ago

Ah that makes sense, and then just have it default to --workspace?

This is the default we have there: https://github.com/emacs-rustic/rustic/blob/17a79c659b0eb304e35705facb114caf61eae2e9/rustic-cargo.el#L90

But yeah, --workspace also seems a reasonable default.

That seems like a different implementation -- from my quick read in rustic you are controlling the arguments to cargo check whereas rust.el is setting the project root once in the function rust-buffer-project and then using it for each sub-command in rust--compile.

In order to keep this change small what I might do instead is create a new custom var cargo-project-locate-arguments and then default that to --workspace

Although now I might just switch to rustic :-D

Cool, note that the installation instruction might need some updates etc. Feel free to send PR's. The fork is not officially announced yet. For more background, here is the information: https://psibi.in/posts/rustic.html

Ah, good to know. I'll keep an eye on it

bradneuman commented 1 month ago

Alternatively I suppose I could remove --workspace there and then add it in to all of the other commands via rust-cargo-default-arguments, but now I need to start thinking about a migration path

psibi commented 1 month ago

whereas rust.el is setting the project root once in the function rust-buffer-project and then using it for each sub-command in rust--compile.

You're right.

Given the situation, I'm starting to lean towards your current implementation.

but now I need to start thinking about a migration path

I'm not sure if it's a good idea to cause such a big breaking change.

bradneuman commented 1 month ago

That's definitely fine with me :)

I haven't used github in a while and I can't seem to figure out how to kick off the CI job again - though it looks like it should be fixed. Am I missing something obvious? (All the internet instructions just say to click some button with a rewind icon but of course I don't have that button)

psibi commented 1 month ago

@bradneuman I have left a comment here: https://github.com/rust-lang/rust-mode/pull/546#discussion_r1600884096

psibi commented 1 month ago

Closing this as it has been addressed by https://github.com/rust-lang/rust-mode/pull/546