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

Accommodate VSCode users with the rust-analyzer extension #846

Merged
merged 6 commits into from
Feb 2, 2024

Conversation

billylanchantin
Copy link
Contributor

Add support for VSCode developers with this extension:

It's the primary LSP implementation for Rust (so far as I can tell). It gives me hover hits, etc. This PR makes it work without having to change directories:

rust-lsp

@billylanchantin
Copy link
Contributor Author

Do we need to do anything else for releases?

@josevalim
Copy link
Member

What if we use the cargo workspace function instead of a symlink? There is a discussion here: rust-lang/rust-analyzer#6007 (comment)

@philss
Copy link
Member

philss commented Feb 2, 2024

Do we need to do anything else for releases?

I think that with the workspace that José suggested, we would need to create a symlink for the target dir of the root of the project. This is because cargo is going to save the files in the root directory, and we are looking to the native/explorer/target directory for the shared lib.
I'm going to check if there is another way to locate the file automatically.

BTW, I tested the workspace config with the following:

[workspace]
resolver = "2"

members = [
  "native/*"
]

[patch.crates-io]
jsonpath_lib = { version = "0.3", git = "https://github.com/ritchie46/jsonpath", branch = "improve_compiled" }

@billylanchantin
Copy link
Contributor Author

billylanchantin commented Feb 2, 2024

@josevalim @philss I tried the EDIT at the top of the Jose's linked comment and it just worked lol. (I swear I looked for such a setting but couldn't find one 😅)

The workspace thing may not be necessary if the settings.json approach works. But IDK enough about Rust to say for sure.

@billylanchantin billylanchantin changed the title Accomodate VSCode users with the rust-analyzer extension Accommodate VSCode users with the rust-analyzer extension Feb 2, 2024
@philss
Copy link
Member

philss commented Feb 2, 2024

@billylanchantin nice! I prefer this way! :D

PS: maybe you need to change your editor config to add newlines at the end of the JSON files. Our .editorconfig file is configured to add new lines. Not sure why your VS Code is not reading that (maybe it needs an extension?).

@billylanchantin
Copy link
Contributor Author

billylanchantin commented Feb 2, 2024

Our .editorconfig file is configured to add new lines. Not sure why your VS Code is not reading that (maybe it needs an extension?).

@philss Yep, that was it! I added an extension and VSCode started adding newlines correctly.

To cement that, I've added an extensions.json that records which extensions we recommend, including the editorconfig extension. We can add more of course, but this is probably fine for now.

Copy link
Member

@philss philss left a comment

Choose a reason for hiding this comment

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

🚢

@billylanchantin billylanchantin merged commit 5dbdbbd into main Feb 2, 2024
3 checks passed
@billylanchantin billylanchantin deleted the bl-vscode-rust-analyzer branch February 2, 2024 17:47
@josevalim
Copy link
Member

I am not the biggest fan of adding editor specific configuration files. Isn't there a general configuration we could use that would work for many editors, such further modifications to editor config?

@billylanchantin
Copy link
Contributor Author

I am not the biggest fan of adding editor specific configuration files.

@josevalim yeah I feel ya. If you have N editors on your team, suddenly you've got N config files floating around.

I see two things we can do:

  1. Add .vscode/ to my .git/info/exclude (aka "private" .gitignore) then remove it from the project.
  2. Take another pass at the workspace thing.

I can do the .git/info/exclude thing either way. That way at least I'm unblocked personally.

Then I/we can take another stab at the workspace approach. That would possibly obviate the need for .vscode/settings.json, meaning future VSCode contributors would be unblocked too.

@billylanchantin
Copy link
Contributor Author

This works for me:

  • Remove .vscode/
  • Add @philss's Cargo.toml
  • Symlink in root:
    • native/explorer/Cargo.lock
    • native/explorer/target

Do we want that instead?

@josevalim
Copy link
Member

We may be able to configure the build directory and skip the symlinks:

https://stackoverflow.com/questions/50364390/how-can-i-specify-a-custom-cargo-output-directory

But perhaps we could move both Cargo.toml and Cargo.lock to the root directory and just keep the source directly inside "native" or inside "src".

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.

3 participants