Skip to content
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

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

Closed
bradneuman opened this issue May 13, 2024 · 10 comments

Comments

@bradneuman
Copy link
Contributor

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 added a commit to bradneuman/rust-mode that referenced this issue May 13, 2024
Adds rust-locate-project-in-workspace custom variable, which controls whether or not to locate the workspace
project using `--workspace` (the default) or not.

In cases where there is only one create, this should make no difference.

The default setting should match the existing behavior.

Github issue rust-lang#545
@bradneuman
Copy link
Contributor Author

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

@psibi
Copy link
Member

psibi commented May 14, 2024

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
Copy link
Contributor Author

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

Although now I might just switch to rustic :-D

@psibi
Copy link
Member

psibi commented May 14, 2024

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
Copy link
Contributor Author

bradneuman commented May 14, 2024

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
Copy link
Contributor Author

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
Copy link
Member

psibi commented May 14, 2024

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
Copy link
Contributor Author

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
Copy link
Member

psibi commented May 17, 2024

@bradneuman I have left a comment here: #546 (comment)

@psibi
Copy link
Member

psibi commented May 29, 2024

Closing this as it has been addressed by #546

@psibi psibi closed this as completed May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants