Skip to content
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.

Sign up for GitHub

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

Jump to bottom

rustdoc-search: show type signature on type-driven SERP #124544

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

rustdoc-search: show type signature on type-driven SERP #124544

wants to merge 5 commits into from
+1,342 −648

Conversation

notriddle
Copy link
Contributor

@notriddle notriddle commented Apr 30, 2024 •
edited
Loading

This is a rewrite of #117112, based on feedback by @aDotInTheVoid.

Part of rust-lang/rust-project-goals#112

Preview

Preview page:

Screenshots:

image

image

image

image

image

image

Rationale

Since the point of this feature is to answer the question "why this a match," it's important that it communicates a few things:

  • How do I use this? This UI shows types as Rustdoc sees them, using Rustdoc's search syntax, rather than the syntax used by normal Rust. It's supposed to act as a built-in, context-sensitive tutorial, since you can type a name in, switch to the In Parameters tab, and see what you could've typed to get a particular result.
  • Which type parameters got matched to other type parameters? This information is found in a popover that you can open to see information about a search result. This makes the visible type signature a strict dump of the search index, providing reasonably clear separation of user input from output.
  • How does the search engine even work, anyway? It's important that this page accurately shows you what unboxing occurred and which mappings got followed. That's why parts of the highlighter are baked into the type unification function: it doesn't just show you where your atoms occur, but actually shows you which ones got matched. A highlighted "where clause" is also found in the About This Result popover.

This change doesn't touch deduplication. That logic needs some adjusting, but is separate enough from this problem to be done in a different PR.

How it works

A particular source of complexity in this PR is caused by storing the names of type parameters. Since this data is only needed after the results come up, loading them gets delayed until after the search is done, so that they can be loaded at the same time as the descriptions.

Future possibilities

This PR is based on #124148. This feature benefits from the type signature index itself being accurate, so we really kinda need full support for Rust's type grammar.

@rustbot
Copy link
Collaborator

rustbot commented Apr 30, 2024

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Apr 30, 2024
@GuillaumeGomez
Copy link
Member

I'll let @aDotInTheVoid confirms it looks good before reviewing the code.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from 888ee4d to 681f0c2 Compare May 16, 2024 04:02
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle marked this pull request as ready for review June 5, 2024 19:05
@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2024

Some changes occurred in GUI tests.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@bors
Copy link
Contributor

bors commented Jun 8, 2024

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

@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from 0feb8de to ee3ee04 Compare June 8, 2024 14:57
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from ee3ee04 to e350324 Compare June 8, 2024 15:45
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from e350324 to f817d33 Compare June 8, 2024 16:28
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from f817d33 to 801c978 Compare June 8, 2024 17:17
@aDotInTheVoid
Copy link
Member

Having played around with this, I really like it. I think the searching side seems really solid and have some small nitpicks on the UI side. Feel free to disregard them, as I'm not great at this, and others on the rustdoc team have put alot more thaught into our UI than I have.

In rough order of importance:

  • The type signature should probably be in monospace. Seeing "Vec<u8> -> String" is much weirder to me than "Vec<u8> -> String"
  • I don't think the "About this result" infobox worth having in most (if not all cases), now that we use the source name for generics. It should defiantly be elided when it only had info like T is T
    • Additionally, I'm not sure how nessesary it it to display the bounds here. Is saying K: HashEq helpfull here
      • Random aside: In the infobox they say K: HashEq, but have K: Hash + Eq
  • Really Small Thing: the (i) for notable traits changes color on highlight.
  • I don't love -> Vec<T> for a function that takes no arguments, but I don't have better ideas.

Thanks again for working on this, I'm really excited about having this!

GuillaumeGomez
GuillaumeGomez reviewed Jun 9, 2024
View reviewed changes
src/librustdoc/html/static/css/rustdoc.css Outdated
@@ -2743,17 +2748,19 @@ Original by Dempfi (https://github.com/dempfi/ayu)
border-right: 1px solid #ffb44c;
}

:root[data-theme="ayu"] .search-results a:hover,
:root[data-theme="ayu"] .search-results a:focus {
:root[data-theme="ayu"] .search-results li:hover > a,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this special case for the ayu theme is really not great, we should try to either make a new variable or "fix" the ayu theme at some point (not for this PR).

@GuillaumeGomez
Copy link
Member

Please also add a test for clicking on the i icon in the search results.

One final thought: I think, from what I understood of the code, that maybe it could be simplified to handle the notable trait part. I'll need to come back to this to ensure I didn't miss something obvious.

@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from d476d19 to 8fe7dd4 Compare June 11, 2024 19:05
@notriddle
Copy link
Contributor Author

Pushed changes to address nits:

  • switch color for ⓘ on hover
  • monospace type signature
  • omit bounds with no highlights and T is T name mappings
  • omit empty info popovers
  • fixed bounds formatting (added missing +s)
  • add more comments to code, minor cleanup

notriddle added a commit to notriddle/rust that referenced this pull request Jul 10, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
0c751b2 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
@notriddle notriddle mentioned this pull request Jul 10, 2024
Open
notriddle added a commit to notriddle/rust that referenced this pull request Jul 11, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
104a4eb 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from b051735 to e0c3562 Compare July 25, 2024 00:32
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from e0c3562 to f8b3e1a Compare July 25, 2024 01:02
notriddle added a commit to notriddle/rust that referenced this pull request Jul 25, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
759df53 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
@notriddle
Copy link
Contributor Author

@camelid Does #127589 resolve the problems with type parameter matching?

@bors
Copy link
Contributor

bors commented Jul 27, 2024

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

@notriddle notriddle force-pushed the notriddle/type-signature-v2 branch from f8b3e1a to b90d43d Compare July 27, 2024 04:33
notriddle added a commit to notriddle/rust that referenced this pull request Jul 27, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
29d228b 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
notriddle added a commit to notriddle/rust that referenced this pull request Aug 13, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
bbdd6e8 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
notriddle added a commit to notriddle/rust that referenced this pull request Aug 13, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
eb5487e 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member

cc @camelid

notriddle added a commit to notriddle/rust that referenced this pull request Sep 25, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
3936dd1 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
notriddle added a commit to notriddle/rust that referenced this pull request Sep 25, 2024
@notriddle
rustdoc-search: simplify rules for generics and type params
f7500c7 
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
@Dylan-DPC Dylan-DPC added S-waiting-on-concerns Status: Awaiting concerns to be addressed by the author S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2024
@bors
Copy link
Contributor

bors commented Oct 27, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

@aDotInTheVoid aDotInTheVoid Awaiting requested review from aDotInTheVoid

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-concerns Status: Awaiting concerns to be addressed by the author S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@notriddle @rustbot @GuillaumeGomez @rust-log-analyzer @bors @aDotInTheVoid @rfcbot @camelid @jsha @Dylan-DPC