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 Rust Analyzer in the install instructions #27

Merged
merged 6 commits into from
Mar 29, 2023

Conversation

CGMossa
Copy link
Contributor

@CGMossa CGMossa commented Mar 25, 2023

UPDATE: This PR now specifies the dependence on Rust Analyzer through the install instructions.

Currently, installing the extension for developers, depends on Rust Analyzer.
That is due to "type": "cargo" is a construction from rust-analyzer.

Instead, this just invokes cargo as a shell command.

@nvarner
Copy link
Owner

nvarner commented Mar 26, 2023

Is this a common problem? My understanding is that effectively all Rust developers on VS Code use rust-analyzer.

@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023

Either this change or the install instructions should contain rust analyzer.
A user was having issues.. They are still having them but I solved this one.

@nvarner
Copy link
Owner

nvarner commented Mar 26, 2023

Recommending installing rust-analyzer would be better, since Rust development without it would be difficult.

As an aside, do you have a link to where someone was having trouble? I don't think I saw it, but I'll offer help if I'm able, and seeing what's hard can drive development.

@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023 via email

@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023

What's the benefit of having the cargo build task depend on rust analyzer rather than this?

@nvarner
Copy link
Owner

nvarner commented Mar 26, 2023

Oh, I see; it's not possible to even run the extension without the RA plugin.

However, using it gives us problem matchers. In general, it should usually provide better editor integration than a command. It probably also works better in strange environments. For instance, I can see someone alias an unusual command to cargo, or leave it out of their path, or something else unexpected. The RA task should still work, but the command line would not.

I think it's still best to pivot this PR into recommending installing RA. It would also be okay if there's an easy way to add the RA problem matcher to the command line task without depending on RA, and the task is renamed to indicate that it's part of the build process for testing the extension, not a recommended way of building the LSP.

@nvarner
Copy link
Owner

nvarner commented Mar 26, 2023

And thanks for providing support!

@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023

Thanks! And no problem.. Unfortunately collaboration requires that someone gets educated sometimes.

@CGMossa CGMossa closed this Mar 26, 2023
@CGMossa CGMossa force-pushed the remove_rust_analyzer_dependence branch from 5a57810 to 893b324 Compare March 26, 2023 08:16
@CGMossa CGMossa reopened this Mar 26, 2023
@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023

@nvarner It should be ready now.

@CGMossa CGMossa changed the title Remove dependence on rust-analyzer Add Rust Analyzer in the install instructions Mar 26, 2023
@jakobhellermann
Copy link
Contributor

You can also add rust-analyzer to vscode extension recommendations, that way it will show a little popup asking you to install rust-analyzer when you first open the project https://tattoocoder.com/recommending-vscode-extensions-within-your-open-source-projects/

@CGMossa
Copy link
Contributor Author

CGMossa commented Mar 26, 2023

Perfect suggestion; I've added it now.

@nvarner
Copy link
Owner

nvarner commented Mar 29, 2023

Thanks! This will help new users.

@nvarner nvarner merged commit 9ca3f28 into nvarner:master Mar 29, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants