-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[rustdoc] Fix type based search #115436
[rustdoc] Fix type based search #115436
Conversation
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha |
// The String is alias name and the vec is the list of the elements with this alias. | ||
// | ||
// To be noted: the `usize` elements are indexes to `items`. | ||
aliases: &'a BTreeMap<String, Vec<usize>>, | ||
} | ||
|
||
struct Paths { |
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 needed to create this type because in case there is no path (for primitive types for example), we don't want the array to finish with a null
element.
This comment has been minimized.
This comment has been minimized.
66f4016
to
36fa557
Compare
Fixed the eslint issue: it was a comma after the last function's argument... |
if path.len() < 2 { | ||
return Paths { ty: *ty, name: path[0], path: None }; | ||
} | ||
let index = mod_paths.get(&join_with_double_colon(&path[..path.len() - 1])); |
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.
How does this work when a function signature contains a type that’s defined in another crate and not inlined into this one anywhere?
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.
It doesn't, good catch. We only get the paths from the cache. We can get the path with def_path_str
. Going to take a look.
Fixed the paths for foreign items as well. I added their paths at the end of the |
That sounds good. @bors r+ rollup=never This will have a perf impact, since it adds more data to the search index. That's expected, and since this is a bug fix, acceptable as long as it isn't huge. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (3ec4b3b): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 629.598s -> 630.213s (0.10%) |
Fixes #114522.
The problem was a bit more tricky than I originally thought it would be: we only kept type ID and generics in short, but as soon as there was a full path in the user query, the element didn't get an ID anymore because the ID map didn't know about
x::y
(although it knew abouty
). So for this first problem, I instead always pass the element name to get the ID.Then a new problem occurred: we actually needed to check if paths matched, otherwise whatever the path, as long as the "end types" match, it's all good. meaning, we needed to add path information, but to do so, we needed it to be added into the search index directly as there was no mapping between
"p"
and"q"
.I hope this explanation makes sense to someone else than me. ^^'
r? @notriddle