-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Rust: expose more rust-analyzer config knobs #18756
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This pull request exposes more rust-analyzer configuration knobs by adding new deserialization functions for vectors and maps, updating config field definitions, and adjusting workspace loading APIs. Key changes include:
- Introducing a new module (deserialize.rs) to handle newline/comma-separated strings.
- Merging duplicate deserialization logic by removing the former deserialize_vec.rs.
- Expanding the configuration options in the Config struct (e.g. sysroot, rustc_src, build_script_command, extra_includes, and proc_macro_server).
- Modifying the cargo configuration construction and workspace loading functions in both config.rs and rust_analyzer.rs.
- Adjusting the procedural macro in macros/src/lib.rs to use the updated deserialization functions.
Changes
File | Description |
---|---|
rust/extractor/src/config/deserialize.rs | Adds new vector and map deserialization visitors and functions for comma/newline separated strings. |
rust/extractor/src/config.rs | Updates the Config struct and related functions to expose additional fields and refactors cargo config logic. |
rust/extractor/macros/src/lib.rs | Updates macro attribute usage to employ the new deserialization functions with adjusted type checks. |
rust/extractor/src/main.rs | Modifies workspace loading code to pass the new LoadCargoConfig parameter. |
rust/extractor/src/rust_analyzer.rs | Adjusts the load_workspace API to remove unused proc_macro_server references. |
rust/extractor/src/config/deserialize_vec.rs | Removes legacy duplicate deserialization logic. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
rust/extractor/src/config.rs:217
- [nitpick] Consider renaming 'join_path_buf' to a name such as 'join_abs_path' for better clarity regarding its purpose of joining an absolute path with a UTF-8 path.
fn join_path_buf(lhs: &AbsPath, rhs: &Path) -> AbsPathBuf {
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just a minor question.
|
||
fn join_path_buf(lhs: &AbsPath, rhs: &Path) -> AbsPathBuf { | ||
let Ok(path) = Utf8PathBuf::from_path_buf(rhs.into()) else { | ||
panic!("non utf8 input: {}", rhs.display()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: does display
work correctly if rhs
contains non-utf8 data? We wouldn't like to cause an additional panic
while trying to display an informative error message.
TODO: expose all or at least some of the new config fields as official extractor options. Depending on how the review goes for this one, I might do that in a follow-up PR.