feat: rename references in intra-doc links - #23030
Conversation
rustbot
added
the
S-waiting-on-review
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| .unwrap_or(s) | ||
| } | ||
|
|
||
| /// Extracts all intra-doc link occurrences from Markdown documentation. |
There was a problem hiding this comment.
| /// Extracts all intra-doc link occurrences from Markdown documentation. | |
| /// Extracts all links from a given markdown text returning the definition text range, link-text | |
| /// and the namespace if known. |
You removed this and this is useful.
| } | ||
|
|
||
| fn broken_link_clone_cb(link: BrokenLink<'_>) -> Option<(CowStr<'_>, CowStr<'_>)> { | ||
| Some((link.reference.clone(), link.reference)) |
There was a problem hiding this comment.
Duplicated with doc_links.rs, you also removed the comment.
| const MARKDOWN_OPTIONS: Options = | ||
| Options::ENABLE_FOOTNOTES.union(Options::ENABLE_TABLES).union(Options::ENABLE_TASKLISTS); |
There was a problem hiding this comment.
Also duplicated with doc_links.rs.
|
Hmm. I made some changes locally but I have a question: should I inlcude intra docs in other usages like go to references? |
|
I think we should start with not. It's reasonable to do, but we'll want a config for it anyway, so not for this PR. |
|
The current implementation is indeed fairly complicated 🤔 However I noticed one issue: FindUsages has a fast path for short inherent associated unctions such as new and from After the refactoring, in order to handle intra-doc links in the normal search loop, rename needs to bypass this fast path when intra-doc link search is enabled. Is that performance trade-off acceptable for this PR or should the fast path also be adapted to handle intra-doc links during rename? |
|
You don't need to disable this, you just need to also search doc comments (and string literals in |
|
Actually, you don't even need an addition search - the existing short associated function path could find doclinks (textually) just as well. |
3 participants
resolve #12583
One point worth noting is that I deliberately added an
include_intra_doc_linksoption to control whether intra-doc links are included in usage search results. For now, I have enabled it only for rename, because I suspect that including documentation links in other features, such as Find References, could make their results too noisy. I have also deliberately left renaming links inside macros unsupported for now.