-
Notifications
You must be signed in to change notification settings - Fork 13k
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 search: allow queries to end in an empty path segment #132569
rustdoc search: allow queries to end in an empty path segment #132569
Conversation
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha |
This comment has been minimized.
This comment has been minimized.
Sounds like a good idea. Should we prioritize fields/variants over methods/functions though? |
591a885
to
38c6073
Compare
we could, and it would be pretty trivial, but it's also pretty easy to do it does look a little odd having them awkwardly in the middle of the list, but that's also the norm for searches like all in all, I have no strong options w.r.t. sorting. |
Well as usual in such cases, let's see what others think about it. |
Thinking about it, is there any reason that we need to address sorting now, instead of in a followup PR? Perhaps variants should be sorted above methods in all situations, not just when the last element is empty? |
This comment has been minimized.
This comment has been minimized.
38c6073
to
eb17234
Compare
deleted the test case that expects the removed error. |
@@ -0,0 +1,6 @@ | |||
const EXPECTED = { |
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.
Instead of this test, can you add a new one in rustdoc-js
instead so we can check exactly what is returned? We have type with very long names so I don't think you need to update the sources for it, just pick a type and ensure its items are listed.
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's not the length of the type that matters, it's the length of the associated item.
but yeah, it could probably use a more extreme testcase.
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.
Did you add the test?
Fair point. Then let's keep this PR as is. Just need an update on the search test and then looks good to me. |
eb17234
to
9af8712
Compare
Alright, added another testcase with the longest item name I could find. This also tests the whole-crate search case. |
This comment has been minimized.
This comment has been minimized.
9af8712
to
b5a9b13
Compare
ah, generics.js uses an exact check... i could write a whole new test, but at the same time, I know I'm just gonna revert my last change for now. I don't actually have the proper stuff installed to run these tests and bless the output. |
☔ The latest upstream changes (presumably #127589) made this pull request unmergeable. Please resolve the merge conflicts. |
b5a9b13
to
8ab783c
Compare
☔ The latest upstream changes (presumably #133039) made this pull request unmergeable. Please resolve the merge conflicts. |
r=me once the conflicts are resolved |
what's the rationale for inlining handleSingleArg? |
The old handleSingleArg handled one "row" of the search index at a time, populating both "In Names" and the "In Params/Return" type-based results. I had to split it in half.
Neither of these halves seemed big enough to give their own function, because the "meat" of both were already mostly in separate functions. |
8ab783c
to
306e07d
Compare
…-empty-v2, r=notriddle rustdoc search: allow queries to end in an empty path segment fixes rust-lang#129707 this can be used to show all items in a module, or all associated items for a type. currently sufferes slightly due to case insensitivity, so `Option::` will also show items in the `option::` module. it disables the checking of the last path element, otherwise only items with short names will be shown r? `@notriddle`
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#131717 (Stabilize `const_atomic_from_ptr`) - rust-lang#132134 (Remove `ResultsVisitable`) - rust-lang#132449 (mark is_val_statically_known intrinsic as stably const-callable) - rust-lang#132569 (rustdoc search: allow queries to end in an empty path segment) - rust-lang#132787 (Unify FnKind between AST visitors and make WalkItemKind more straight forward) - rust-lang#132832 (Deny capturing late-bound ty/const params in nested opaques) - rust-lang#133097 (Opt out TaKO8Ki from review rotation for now) r? `@ghost` `@rustbot` modify labels: rollup
…nd-empty-v2, r=notriddle rustdoc search: allow queries to end in an empty path segment fixes rust-lang#129707 this can be used to show all items in a module, or all associated items for a type. currently sufferes slightly due to case insensitivity, so `Option::` will also show items in the `option::` module. it disables the checking of the last path element, otherwise only items with short names will be shown r? ``@notriddle``
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#131717 (Stabilize `const_atomic_from_ptr`) - rust-lang#132134 (Remove `ResultsVisitable`) - rust-lang#132449 (mark is_val_statically_known intrinsic as stably const-callable) - rust-lang#132569 (rustdoc search: allow queries to end in an empty path segment) - rust-lang#132787 (Unify FnKind between AST visitors and make WalkItemKind more straight forward) - rust-lang#132832 (Deny capturing late-bound ty/const params in nested opaques) - rust-lang#133097 (Opt out TaKO8Ki from review rotation for now) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132569 - lolbinarycat:rustdoc-search-path-end-empty-v2, r=notriddle rustdoc search: allow queries to end in an empty path segment fixes rust-lang#129707 this can be used to show all items in a module, or all associated items for a type. currently sufferes slightly due to case insensitivity, so `Option::` will also show items in the `option::` module. it disables the checking of the last path element, otherwise only items with short names will be shown r? `@notriddle`
Rollup of 8 pull requests Successful merges: - rust-lang#131717 (Stabilize `const_atomic_from_ptr`) - rust-lang#132449 (mark is_val_statically_known intrinsic as stably const-callable) - rust-lang#132569 (rustdoc search: allow queries to end in an empty path segment) - rust-lang#133029 (ABI checks: add support for some tier3 arches, warn on others.) - rust-lang#133093 (Let chains tests) - rust-lang#133097 (Opt out TaKO8Ki from review rotation for now) - rust-lang#133116 (stabilize const_ptr_is_null) - rust-lang#133126 (alloc: fix `String`'s doc) r? `@ghost` `@rustbot` modify labels: rollup
… r=GuillaumeGomez rustdoc-search: add standalone trailing `::` test Follow up for rust-lang#132569 r? `@GuillaumeGomez`
Rollup merge of rust-lang#133133 - notriddle:notriddle/trailing-test, r=GuillaumeGomez rustdoc-search: add standalone trailing `::` test Follow up for rust-lang#132569 r? `@GuillaumeGomez`
fixes #129707
this can be used to show all items in a module,
or all associated items for a type.
currently sufferes slightly due to case insensitivity, so
Option::
will also show items in theoption::
module.it disables the checking of the last path element, otherwise only items with short names will be shown
r? @notriddle