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

Changes to support rust-analyzer in a limited fashion at least #2029

Closed
wants to merge 3 commits into from

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Aug 29, 2024

There are a few issues (mostly stemming from us lying about the architecture), but I think the benefits outweigh them. And it's still possible to disable rust-analyzer either repo-wise, or per-package, using excludeDirs.

Unfortunately, the cause of the proc-macro pains of this PR is unclear

The biggest change is possibly that now everything is placed into the workspace's target folder, so examples may clash and documentation paths changed.

@bugadani bugadani added the skip-changelog No changelog modification needed label Aug 29, 2024
@bugadani bugadani force-pushed the ra branch 2 times, most recently from ffaeb4e to f1041a4 Compare August 29, 2024 11:56
@jessebraham
Copy link
Member

I had just assumed people were writing their own config files, certainly been the case here.

FYI we previously had VS Code configuration in the repo but it ended up being a bit of a pain in the ass, it periodically breaks due to rust-analyzer changes, and people tend to check in various different versions of the configuration to version control. Not saying I'm rejecting this, but keep in mind that it was removed for a reason.

@bugadani
Copy link
Contributor Author

Please note that I'm not checking in any actual config files. I'm trying to provide a quick "rename this"-style template that people can then customize.

There are a few unresolved problems I'm still papering over, but I believe that even a partially functional rust-analyzer has a ton of added benefits.

@bugadani bugadani force-pushed the ra branch 5 times, most recently from 9eb8d63 to c1daa3b Compare August 29, 2024 13:48
@bugadani
Copy link
Contributor Author

I'm slightly concerned by the change in output paths. Whatever I took out of excluded now places its artifacts in ./target instead of ./<package>/target.

@@ -73,15 +73,15 @@ embedded-hal = ["dep:embedded-hal", "dep:embedded-hal-nb"]
embedded-io = ["dep:embedded-io"]

[[example]]
name = "blinky"
name = "lp_blinky"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed because examples are now placed into the same target folder (the workspace's) regardless of the source package.

.join("target")
.join(target)
.join("doc"),
.join("doc")
.join(package.to_string().replace('-', "_")),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change means we can't generate documentation for the packages excluded from the workspace. Not sure if it matters, I don't think we want to do that for the xtensa-lx packages or hil tests at this moment.

@bugadani bugadani changed the title [WIP] Rust-analyzer Changes to support rust-analyzer Aug 30, 2024
@bugadani bugadani marked this pull request as ready for review August 30, 2024 20:38
@bugadani bugadani changed the title Changes to support rust-analyzer Changes to support rust-analyzer in a limited fashion at least Aug 30, 2024
@bugadani bugadani requested a review from MabezDev September 2, 2024 14:22
@bugadani bugadani closed this Sep 5, 2024
@bugadani bugadani deleted the ra branch September 5, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants