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 warning about rust-analyzer not working if you clone and use the repo directly #2131

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MadLittleMods
Copy link

@MadLittleMods MadLittleMods commented Oct 14, 2024

Add warning about rust-analyzer not working if you clone and use the repo directly.

Yes, you are right, if you just clone the repository and try to edit the exercises, the language server will not work. This is one downside of the current approach. But this only affects developing exercises.

The new method of doing Rustlings is to install Rustlings using cargo install rustlings (not published yet), then running rustlings init. No repo cloning happens. Instead, the directory rustlings/ will be created where you find the exercises. The language server works there out of the box :)

I need to add a warning when people try to work on the exercises from the repository. Thanks pointing this out.

-- @mo8it, #1935 (comment)

Reproduction steps

Personally, I also fell into this trap since I would prefer to just run the thing from source than install something on my system.

  1. Clone the repo: git clone [email protected]:rust-lang/rustlings.git
  2. Start Rustlings: cargo run
  3. Open VSCode and notice that rust-analyzer and lsp hints don't pop up
  4. Google and find StackOverflow answers that mention rustlings lsp to setup lsp support
  5. But rustlings lsp doesn't exist anymore and the changelog mentions that it should be supported out of the box. It's also confusing because the rustlings repo has it's own Cargo.toml with include = [... "/exercises/", ...] which to my newbie Rust eyes, seems like it might cover it and makes me think, this is supposed to work.
  6. Eventually, I saw Use Cargo.toml instead of rust-project.json #1935 linked somewhere and it mentions this exact problem

Dev notes

…e repo directly

> Yes, you are right, if you just clone the repository and try to edit the exercises, the language server will not work. This is one downside of the current approach. But this only affects developing exercises.
>
> The new method of doing Rustlings is to install Rustlings using `cargo install rustlings` (not published yet), then running `rustlings init`. No repo cloning happens. Instead, the directory `rustlings/` will be created where you find the exercises. The language server works there out of the box :)
>
> I need to add a warning when people try to work on the exercises from the repository. Thanks pointing this out.
>
> -- @mo8it, rust-lang#1935 (comment)

Other references:

 - Previous `rustlings lsp` command: rust-lang#1026
 - The changelog says "LSP support out of the box", https://github.com/rust-lang/rustlings/blob/main/CHANGELOG.md#lsp-support-out-of-the-box
@mo8it
Copy link
Contributor

mo8it commented Oct 16, 2024

I agree that we should add something to the README for this, but the addition should tell people to not clone the repository and instead follow the instructions above. A simple warning should be enough. Something like this:

"Don't try to clone the repository to do the exercises! Rust-Analyzer won't work in that case. Please follow the instructions above instead."

@@ -45,6 +45,18 @@ cargo install rustlings

</details>

> [!CAUTION]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used caution but feel free to nit to warning or any other.

@@ -45,6 +45,18 @@ cargo install rustlings

</details>

> [!CAUTION]
> Don't try to clone the repository to do the exercises! `rust-analyzer` won't work in that case. Please follow the instructions above instead.
Copy link
Author

@MadLittleMods MadLittleMods Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to your exact wording ✅

I've kept the extra "Why?" background in a collapsible section since it feels like it adds a little bit of something that makes it more understandable and convincing. Happy to remove to get this shipped if you prefer.

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.

2 participants