-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix Support #[doc = include_str!(...)] in hover documentation #11137 #21303
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
base: master
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.
Pull request overview
This PR adds support for #[doc = include_str!(...)] attributes in hover documentation. Previously, rust-analyzer would silently ignore these attributes when displaying hover information; now it resolves the referenced file, reads its contents, and includes it in the documentation output while preserving the original formatting.
Key changes:
- Extended
DocsSourceMapLineto track file origins and indent-trimming preferences per line - Added logic to detect and process
include_str!macro calls in doc attributes - Updated the VFS to watch Markdown files so they can be resolved and their changes tracked
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/load-cargo/src/lib.rs | Adds "md" extension to watched file types so Markdown files used in include_str! are tracked by the VFS |
| crates/ide/src/hover/tests.rs | Adds regression test verifying that include_str! documentation appears in hover output |
| crates/hir-def/src/attrs.rs | Core implementation: adds file origin tracking, extend_with_doc_include method, preserves indentation for included files, updates all call sites and tests to accommodate new structure fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
(Haven't reviewed this yet, commenting about the approach).
I'm conflicted about this. One one hand, this is an improvement. On the other hand, it's a hack and a heuristic - the correct approach will be to expand macros while collecting docs, but that will require much more changes, an infrastructure for macro calls in attributes, and probably best to discuss the design beforehand. Also, this heuristic will fail if include_str!(), is shadowed, or used by its fully-qualified name.
@rust-lang/rust-analyzer thoughts?
crates/hir-def/src/attrs.rs
Outdated
| db: &dyn DefDatabase, | ||
| variant: VariantId, | ||
| mut field_attrs: impl FnMut(&CfgOptions, InFile<ast::AnyHasAttrs>) -> T, | ||
| mut field_attrs: impl FnMut(&CfgOptions, InFile<ast::AnyHasAttrs>, Crate) -> T, |
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.
There is definitely no reason for this function to take a Crate, the closure can capture it.
|
Yea not too fond of it either, but given that std uses this so much for primitives we might just need to accept this one |
dont add crate
a5449bd to
12f1121
Compare
fix #11137
Docs now tracks file origins per line (with optional HirFileId) and an indent-trimming flag so we can map included Markdown back to its source file without mangling formatting.
Added logic in doc extraction to detect include_str! attributes, resolve the referenced file via AnchoredPath, read its contents, and append it to the aggregated docs while skipping indent stripping.
Updated helpers, tests, and data structures accordingly (including a new unit test ensuring include text preserves indentation).
Added a hover regression test proving that include_str! docs show up in IDE output.