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

Please add included fix so this repo works for rust analyzer users #1016

Closed
David-Else opened this issue Jun 5, 2022 · 11 comments
Closed

Please add included fix so this repo works for rust analyzer users #1016

David-Else opened this issue Jun 5, 2022 · 11 comments

Comments

@David-Else
Copy link

https://gist.github.com/jackos/7332fa8ab0b67a87f382fd566696f412 allows this repo to function correctly with rust analyser and the default Neovim LSP setup, is there a reason it is not included? Thanks.

#443
rust-lang/rust-analyzer#4477

@shadows-withal
Copy link
Member

@David-Else
Copy link
Author

@diannasoreil I am using Neovim, there is no instructions at https://github.com/rust-lang/rustlings#enabling-rust-analyzer how to make rust-analyzer work other than with vs-code and IntelliJ-based editors.

The README says to curl -L https://raw.githubusercontent.com/rust-lang/rustlings/main/install.sh | bash to install and then it works. It doesn't work.

Your link says:

To enable rust-analyzer, you'll need to make Cargo build the project with the exercises feature, which will automatically include the exercises/ subfolder in the project.

I read https://blog.rust-lang.org/2022/02/21/rust-analyzer-joins-rust-org.html , so it is official now? Why not just have 'the exercises feature' as default?

Thanks, I am new so don't have any context to why things are as they are :)

@shadows-withal
Copy link
Member

I am using Neovim, there is no instructions at https://github.com/rust-lang/rustlings#enabling-rust-analyzer how to make rust-analyzer work other than with vs-code and IntelliJ-based editors.

I don't use Vim myself, so I don't know how it would work specifically, but the gist is that you need to run cargo with the --all-features flag somehow (at least, that's easiest).

The README says to curl -L https://raw.githubusercontent.com/rust-lang/rustlings/main/install.sh | bash to install and then it works. It doesn't work.

We don't advertise out-of-the box LSP support ;)

I read https://blog.rust-lang.org/2022/02/21/rust-analyzer-joins-rust-org.html , so it is official now? Why not just have 'the exercises feature' as default?

Yes, rust-analyzer is officially adopted, but that doesn't mean that we, as another official Rust project, have to use it. A pretty significant portion of people who do this course will likely not (want) to use a LSP. We encourage the manual learning process of using rustlings watch, modifying the exercise file, and then seeing what the compiler outputs. LSP encourages a way faster feedback cycle, which I personally don't believe is best for someone learning Rust. That's roughly why it's not turned on by default.

Hopefully that answers some questions!

@David-Else
Copy link
Author

@diannasoreil Thanks for the info, it was very helpful!

If you did include the exercises/ subfolder in the project so rust-analyzer worked, would that negatively affect any users? If not, then I think it should be included as a matter of urgency.

A pretty significant portion of people who do this course will likely not (want) to use a LSP.

In my experience the majority of people will have their editor set up running an LSP (most likely rust-analyzer in Rust), and if this is the case then they will be greeted by an error in rustlings. The error does not explain the problem or link to any issue, so is is a bad user experience that will put people off. If it would just work out of the box with rust-analyzer enabled it would be much better, and people can always turn the LSP off if they advocate your approach of no LSP for learning.

I think the LSP is part of programming now and I would not work without it on, learning how it works for each language is part of learning how to understand the language.

Well that is just my opinion anyway, thanks for this great project, cheers!

@paveloom
Copy link

paveloom commented Jun 8, 2022

I just cloned rustlings for the first time, compiled it, and then added the exercise feature as the default one in Cargo.toml:

[features]
default = ["exercises"]
exercises = []

I think this is what should be recommended in the README instead of enabling a feature globally. Adding the proposed rust-project.json file will solve the issue at the moment, but it inevitably will become a burden to maintain. Just my two cents here.

@jackos
Copy link
Contributor

jackos commented Jun 16, 2022

@David-Else I've added a PR here: #1026 to add a command rustlings lsp after a request from @diannasoreil. This will generate the file, and I also put a section in the initial welcome message and updated the readme, I agree this is important for newcomers to Rust.

For your question on negatives on dropping a rust-project.json in the root as a default, the only one I can think of is that if you're working on the source code it will override the cargo configuration which would be frustrating for someone who didn't understand why, it's not a well-known feature of rust-analyzer.

I've tested and it's working on neovim, vs-code and helix.

@David-Else
Copy link
Author

@jackos @diannasoreil Thanks! This is great news :)

@exdx
Copy link
Contributor

exdx commented Jun 17, 2022

For new users like myself (I just happened to install rustlings today) the documentation points to the new rustlings lsp command, but since the recommended installation method via install.sh relies on the latest tagged release, it's not yet available for users. For others who find themselves in this situation, the previously suggested method works on VSCode:

  • Add a .vscode/settings.json file to the root of the project with the following in it
{
    "rust-analyzer.cargo.features": ["exercises"]
}
  • Restart rust-analyer (it should prompt automatically, if not go to View > Command Palette > Rust-Analyzer: Restart server

After that the exercises should be indexed by rust-analyzer. This is just a temporary workaround until the next release of rustlings becomes available with the new lsp command.

@shadows-withal
Copy link
Member

Aye, I'm planning to push a new release in the next days.

@alex-ter
Copy link

@shadows-withal, given that the lsp subcommand is available now, should this one be closed?

@shadows-withal
Copy link
Member

Sounds good.

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

6 participants