Skip to content

rustdoc: allow searches to match against both type and name #131852

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lolbinarycat
Copy link
Contributor

repurposes existing syntax that previously had a nonsese meaning.

now fn:add, u8 -> u8 searches for fn items with "add" in the name, that take a u8 argument and return a u8.

the kind is included in anticipation that type based searches will soon work on items other than functions and methods.

fixes #131130

r? @notriddle

@rustbot
Copy link
Collaborator

rustbot commented Oct 17, 2024

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 17, 2024
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

cleaned up the formatting a bit, hopefully this satisfies tidy

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-type-and-name branch from 6ff6aed to 8fdad93 Compare October 19, 2024 20:29
@notriddle
Copy link
Contributor

I can't say that I like that comma. If it's tough to get rid of it (I think it would be, since it would require infinite lookahead to distinguish fn:a b, c from fn:a b), here's an alternative suggestion:

Intra-doc links use a disambiguator that's separated from the item itself with an @ instead of a :, as in fn@add. Perhaps that could be the syntax for the extraNameElem, so that the sample query becomes fn@add u8 -> u8.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

I can't say that I like that comma. If it's tough to get rid of it (I think it would be, since it would require infinite lookahead to distinguish fn:a b, c from fn:a b), here's an alternative suggestion:

why do we even allow spaces in item names?

@notriddle
Copy link
Contributor

notriddle commented Oct 20, 2024

@lolbinarycat #108537 was when that was added. The idea was that name-based search worked well when space was used to separate path components, because it's easier than ::

@lolbinarycat
Copy link
Contributor Author

ah, that makes sense. we should probably document that behavior somewhere.

i mean... if we're going treat spaces as :: in that case, we should probably do it here too, since this does support path-based search. i think adding less special cases to the syntax is a good thing, and it would be weird if fn:vec p worked but fn:vec p -> usize didn't.

ultimately, it might not be the "prettiest", but i think keeping the syntax easy to learn and understand is the most important thing.

@notriddle
Copy link
Contributor

ah, that makes sense. we should probably document that behavior somewhere.

https://doc.rust-lang.org/nightly/rustdoc/read-documentation/search.html#search-by-name is supposed to describe it.

@bors
Copy link
Collaborator

bors commented Nov 11, 2024

☔ The latest upstream changes (presumably #127589) made this pull request unmergeable. Please resolve the merge conflicts.

repurposes existing syntax that previously had a nonsese meaning.

now `fn:add, u8 -> u8` searches for fn items with "add" in the name,
that take a `u8` argument and return a `u8`.

the kind is included in anticipation that type based searches will
soon work on items other than functions and methods.

fixes rust-lang#131130
@lolbinarycat lolbinarycat force-pushed the rustdoc-search-type-and-name branch from 8fdad93 to 5caf9c0 Compare November 11, 2024 18:56
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.......... (130/134)
...F       (134/134)


/checkout/tests/rustdoc-gui/search-corrections.goml search-corrections... FAILED
[ERROR] `tests/rustdoc-gui/search-corrections.goml` line 100: ".error" not found: for command `assert-css: (".error", {
    "display": "block"
})`
[ERROR] `tests/rustdoc-gui/search-corrections.goml` line 103: ".error" not found: for command `assert-text: (
    ".error",
    "Query parser error: \"Generic type parameter NotableStructWithLongNamr does not accept generic parameters\"."

Error: ()
Build completed unsuccessfully in 0:03:20
  local time: Mon Nov 11 19:48:55 UTC 2024

@lolbinarycat
Copy link
Contributor Author

Another approach would be adding proper boolean combinators to rustdoc-search, that way we could just do add & u8 -> u8, however, that would be significantly more involved, and might have a performance overhead (we would have to bypass the trie for more complex searches)

@bors
Copy link
Collaborator

bors commented Jan 17, 2025

☔ The latest upstream changes (presumably #135615) made this pull request unmergeable. Please resolve the merge conflicts.

@lolbinarycat
Copy link
Contributor Author

Now that type-based search does actually work for non-function items, do we want to revisit this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc search: allow combining a type based search with a name based search
5 participants