-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[rustdoc] Make aliases search support partial matching #143988
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
Conversation
r? @notriddle rustbot has assigned @notriddle. Use |
Some changes occurred in HTML/CSS/JS. |
@@ -116,7 +116,7 @@ pub(crate) fn build_index( | |||
// Set up alias indexes. | |||
for (i, item) in cache.search_index.iter().enumerate() { | |||
for alias in &item.aliases[..] { | |||
aliases.entry(alias.as_str().to_lowercase()).or_default().push(i); | |||
aliases.entry(alias.to_string()).or_default().push(i); |
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.
Since there is no longer any transformation taking place, and since aliases
doesn't seem to escape the function (based on the fact CrateData
is a struct defined within this function and it borrows the aliases variable), couldn't aliases have its type changed to BTreeMap<&str, Vec<usize>>
to save a bunch of allocations?
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.
Problem is with serde
and borrow checker here. If I use &str
, we can't do:
let mut search_index = std::mem::take(&mut cache.search_index);
just below. Other solution would be to keep Symbol
instead, but Serialize
is not implemented on it, so not possible either. Fixing this would require a lot of code, so better keep it for later optimization.
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.
Is this worth a FIXME comment or is this too small of an optimization?
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.
I don't think it's worth it. Also I expect you will gove it a try soon enough. 😄
7c21fbf
to
c079c96
Compare
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.
Reviewed everything except the changes to tester.js
, only thing I can think of is putting a doc comment on Row.name
to explain that it is the the name of the item or alias, since this does change its semantics, everything else looks pretty good.
Thanks for the review! @bors r=lolbinarycat rollup |
…lbinarycat [rustdoc] Make aliases search support partial matching Fixes rust-lang#140782. To make this work, I moved aliases into the `searchIndex` like any other item. It links to the "original" item with a new `original` field. No so great part is that we need to have some fields like `bitIndex` to be set on the alias to make the description load to work but I consider it minor enough to be ok. This PR voluntarily doesn't handle de-prioritization of aliases as `@lolbinarycat` wished to work on this so I'll leave them this part. 😉 cc `@lolbinarycat`
…lbinarycat [rustdoc] Make aliases search support partial matching Fixes rust-lang#140782. To make this work, I moved aliases into the `searchIndex` like any other item. It links to the "original" item with a new `original` field. No so great part is that we need to have some fields like `bitIndex` to be set on the alias to make the description load to work but I consider it minor enough to be ok. This PR voluntarily doesn't handle de-prioritization of aliases as ``@lolbinarycat`` wished to work on this so I'll leave them this part. 😉 cc ``@lolbinarycat``
Rollup of 12 pull requests Successful merges: - #141260 (Allow volatile access to non-Rust memory, including address 0) - #143604 (Stabilize `const_float_round_methods`) - #143833 (Ban projecting into SIMD types [MCP838]) - #143988 ([rustdoc] Make aliases search support partial matching) - #144078 (Fix debuginfo-lto-alloc.rs test) - #144111 (Remove deprecated `MaybeUninit` slice methods) - #144116 (Fixes for LLVM 21) - #144134 (Cleanup unicode table gen) - #144142 (Add implicit sized bound to trait ascription types) - #144148 (Remove pretty print hack for async blocks) - #144169 (interpret: fix TypeId pointers being considered data pointers) - #144196 (Initialize mingw for the runner's user) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 11 pull requests Successful merges: - #141260 (Allow volatile access to non-Rust memory, including address 0) - #143604 (Stabilize `const_float_round_methods`) - #143988 ([rustdoc] Make aliases search support partial matching) - #144078 (Fix debuginfo-lto-alloc.rs test) - #144111 (Remove deprecated `MaybeUninit` slice methods) - #144116 (Fixes for LLVM 21) - #144134 (Cleanup unicode table gen) - #144142 (Add implicit sized bound to trait ascription types) - #144148 (Remove pretty print hack for async blocks) - #144169 (interpret: fix TypeId pointers being considered data pointers) - #144196 (Initialize mingw for the runner's user) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143988 - GuillaumeGomez:alias-inexact, r=lolbinarycat [rustdoc] Make aliases search support partial matching Fixes #140782. To make this work, I moved aliases into the `searchIndex` like any other item. It links to the "original" item with a new `original` field. No so great part is that we need to have some fields like `bitIndex` to be set on the alias to make the description load to work but I consider it minor enough to be ok. This PR voluntarily doesn't handle de-prioritization of aliases as ```@lolbinarycat``` wished to work on this so I'll leave them this part. 😉 cc ```@lolbinarycat```
Rollup of 11 pull requests Successful merges: - rust-lang/rust#141260 (Allow volatile access to non-Rust memory, including address 0) - rust-lang/rust#143604 (Stabilize `const_float_round_methods`) - rust-lang/rust#143988 ([rustdoc] Make aliases search support partial matching) - rust-lang/rust#144078 (Fix debuginfo-lto-alloc.rs test) - rust-lang/rust#144111 (Remove deprecated `MaybeUninit` slice methods) - rust-lang/rust#144116 (Fixes for LLVM 21) - rust-lang/rust#144134 (Cleanup unicode table gen) - rust-lang/rust#144142 (Add implicit sized bound to trait ascription types) - rust-lang/rust#144148 (Remove pretty print hack for async blocks) - rust-lang/rust#144169 (interpret: fix TypeId pointers being considered data pointers) - rust-lang/rust#144196 (Initialize mingw for the runner's user) r? `@ghost` `@rustbot` modify labels: rollup
…lbinarycat [rustdoc] Make aliases search support partial matching Fixes rust-lang#140782. To make this work, I moved aliases into the `searchIndex` like any other item. It links to the "original" item with a new `original` field. No so great part is that we need to have some fields like `bitIndex` to be set on the alias to make the description load to work but I consider it minor enough to be ok. This PR voluntarily doesn't handle de-prioritization of aliases as ```@lolbinarycat``` wished to work on this so I'll leave them this part. 😉 cc ```@lolbinarycat```
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#141260 (Allow volatile access to non-Rust memory, including address 0) - rust-lang#143604 (Stabilize `const_float_round_methods`) - rust-lang#143988 ([rustdoc] Make aliases search support partial matching) - rust-lang#144078 (Fix debuginfo-lto-alloc.rs test) - rust-lang#144111 (Remove deprecated `MaybeUninit` slice methods) - rust-lang#144116 (Fixes for LLVM 21) - rust-lang#144134 (Cleanup unicode table gen) - rust-lang#144142 (Add implicit sized bound to trait ascription types) - rust-lang#144148 (Remove pretty print hack for async blocks) - rust-lang#144169 (interpret: fix TypeId pointers being considered data pointers) - rust-lang#144196 (Initialize mingw for the runner's user) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #140782.
To make this work, I moved aliases into the
searchIndex
like any other item. It links to the "original" item with a neworiginal
field. No so great part is that we need to have some fields likebitIndex
to be set on the alias to make the description load to work but I consider it minor enough to be ok.This PR voluntarily doesn't handle de-prioritization of aliases as @lolbinarycat wished to work on this so I'll leave them this part. 😉
cc @lolbinarycat