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

feat: Allow adding extra cargo args when running build scripts #14328

Merged
merged 2 commits into from
Mar 12, 2023

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Mar 11, 2023

Closes #14315

Not sure if we want to do it like this or to add an extra config key, though.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 11, 2023
@ayush-india
Copy link

Waiting for the check to be completed
Thanks a lot mate

@lnicola
Copy link
Member Author

lnicola commented Mar 11, 2023

It's waiting for a review, I don't know if this is the best approach.

@ayush-india
Copy link

Does not matter to me i am anyway going to complie it from src and use it

@Veykril
Copy link
Member

Veykril commented Mar 12, 2023

We should add a separate key for that instead and have the check one inherit it like we do with the other configs for these imo

@ayush-india
Copy link

ayush-india commented Mar 12, 2023

@Veykril but for that apporach we need to write all the extra args by hand.

@Veykril
Copy link
Member

Veykril commented Mar 12, 2023

I'm not sure I understand what you mean, instead of setting rust-analyzer.check.extraArgs you'd be setting rust-analyzer.cargo.extraArgs (or both).

@ayush-india
Copy link

@Veykril i cannot find anything named rust-analyzer.cargo.extraArgs.

Sorry if its sounds dumb.

I'm not sure I understand what you mean

The issue i had was when i setted rust-analyzer.check.extraArgs to -r (as you can see in the issue) the problem i had was it made both a debug and release dir. And this pull should fix it (btw i am not a rust expert).

@Veykril
Copy link
Member

Veykril commented Mar 12, 2023

@Veykril i cannot find anything named rust-analyzer.cargo.extraArgs.

It doesn't exist because thats what I am proposing in adding, the check settings should not affect build script building

@lnicola
Copy link
Member Author

lnicola commented Mar 12, 2023

Of course it exists, it's here.

@ayush-india
Copy link

ayush-india commented Mar 12, 2023

@lnicola and @Veykril So it means that i can now use it by
For neovim
local on_attach = require("plugins.configs.lspconfig").on_attach local capabilities = require("plugins.configs.lspconfig").capabilities local lspconfig = require "lspconfig" lspconfig.rust_analyzer.setup { on_attach = on_attach, settings = {["rust-analyzer"] = {cargo = {extraArgs = {"-r"}}}} }
Vscode
rust-analyzer.cargo.extraArgs = ["-r"]

When i clone @lnicola rust-analyzers repo and complie his build-script-extra-args branch

Right, correct me if i am wrong

@lnicola
Copy link
Member Author

lnicola commented Mar 12, 2023

Yes.

@ayush-india
Copy link

Just a random though: (till when will this feature get added to the "main" rust-analyzer)

@ayush-india
Copy link

Idk why i am writing this:

But if i use check.extraArgs it does not work but if i use cargo.extraArgs it does

Thanks mate it fixed my problem (My thinkpad is alive again)

@ayush-india
Copy link

But if i use check.extraArgs it does not work but if i use cargo.extraArgs it does

@lnicola Does not work if i restart my editor.

And then i have to recomplie the reelase dir.

And then it works

@lnicola
Copy link
Member Author

lnicola commented Mar 12, 2023

Not sure I understand, which parameter are you setting?

@ayush-india
Copy link

@lnicola After my indepth research "lol" i find that

When i use your ra it takes around 40s-90s to open ra after a reload (after the first cargo check -r ) but using the stock takes 30s .

But does not fix my problem (makes a debug and rlease dir but your ra does not).

and also takes around 9-11s to get a hover btw does not bother me. I just want only the release dir which your ra provides me.

Again Thanks mate

@ayush-india
Copy link

Just a random though: (till when will this feature get added to the "main" rust-analyzer)

@lnicola can you also tell me this

@lnicola
Copy link
Member Author

lnicola commented Mar 12, 2023

When i use your ra it takes around 40s-90s to open ra after a reload (after the first cargo check -r ) but using the stock takes 30s .

cargo check -r will be slower (8.45s vs. 8.39s on my system 🙄). Is it also slow on a second load (it shouldn't be)? What if you comment out the setting?

But does not fix my problem (makes a debug and rlease dir but your ra does not).

Yes, that's the point of this branch, to allow you to add that -r there.

and also takes around 9-11s to get a hover btw does not bother me. I just want only the release dir which your ra provides me.

Should only happen on the first hover. There's no reason why my branch would be slower unless you've actually setting -r there, which makes the project load slower the first time. That is, unless you've built RA in the debug/dev profile, but you probably didn't.

Just a random though: (till when will this feature get added to the "main" rust-analyzer)

Whenever Veykril has a chance to look again at this PR. If it's today, it will be even be in the version released tomorrow.

@ayush-india
Copy link

ayush-india commented Mar 12, 2023

Thanks mate you have now answered all my question

Hoping that Veykril will look at this PR.

The rust community is (really really)*9999 helpful. Just hoped that every communtiy was this helpful. And also one more reason to love rust

@Veykril
Copy link
Member

Veykril commented Mar 12, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 12, 2023

📌 Commit c3864eb has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 12, 2023

⌛ Testing commit c3864eb with merge f1e51af...

@bors
Copy link
Collaborator

bors commented Mar 12, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing f1e51af to master...

@bors bors merged commit f1e51af into rust-lang:master Mar 12, 2023
@lnicola lnicola deleted the build-scripts-extra-args branch March 12, 2023 15:34
@lnicola lnicola changed the title fix: Pass flycheck extra args when running build scripts feat: Allow adding extra cargo args when running build scripts Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lsp not working in cargo check -r
5 participants