rustdoc: when running a function-signature search, tweak the tab bar by notriddle · Pull Request #96590 · rust-lang/rust

notriddle · 2022-04-30T21:15:21Z

Before

After

rust-highfive · 2022-04-30T21:15:24Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

rust-highfive · 2022-04-30T21:15:25Z

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

src/test/rustdoc-gui/search-tab-change-title-fn-sig.goml

src/librustdoc/html/static/js/search.js

GuillaumeGomez · 2022-05-02T13:08:32Z

Considering it's a UI/UX change, I'd like to have @jsha and @Manishearth's opinion before going further. Personally I think it'd be a nice improvement.

Manishearth · 2022-05-02T14:44:51Z

I think it's a good call!

jsha · 2022-05-02T15:29:10Z

Agreed! This is something that's bugged me for a while. Thanks for fixing it.

GuillaumeGomez · 2022-05-02T15:35:58Z

Ok so just remains to add a differentiation between returned value and "in function signature" and then let's go!

GuillaumeGomez · 2022-05-02T18:32:25Z

There is a JS error when the results are displayed:

The left/right arrows should be disabled in case there is only one column displayed, otherwise it looks like this (no "selected" header):

Please add a check in your GUI test for the left/right arrows in such case to ensure that it doesn't change the selected header.

notriddle · 2022-05-03T00:39:19Z

Okay, I've got the keyboard shortcut thing fixed.

src/librustdoc/html/static/js/search.js

src/test/rustdoc-gui/search-tab-change-title-fn-sig.goml

GuillaumeGomez · 2022-05-03T15:10:20Z

I can confirm it's working locally as expected. 👍

GuillaumeGomez · 2022-05-03T15:30:29Z

And I still have a JS error:

notriddle · 2022-05-03T16:23:20Z

I see. The error came up when there was a parse error, because the code doesn't work well when there's no tabs at all.

To avoid adding a bunch of special-cases just for the parse error screen, I've added an "In Names" tab (conceptually, this is where doc aliases go), fixing the console log error.

GuillaumeGomez · 2022-05-03T19:45:20Z

Just one last weird thing:

It's a bit of strange rendering with doc aliases. Not a blocker though. You can try it with:

#[doc(alias = "->a")]
pub struct Foo;

Just confirming it's not an issue then let's go!

What do you think of this @Manishearth @jsha ? I personally think it's fine but asking just in case.

notriddle · 2022-05-05T00:18:15Z

I get why this seems strange, but

It seems unavoidable, given how doc aliases work. They're allowed to have arbitrary text in them, which means they're allowed to overlap with search syntax. Disallowing this would be backwards-incompatible, and redesigning this would mean redesigning the search feature pretty heavily just for a niche feature like allowing ->a as a doc alias.
It seems like you're holding it wrong. This is a problem because the doc alias has valid syntax for a function return value, but it doesn't actually describe a function or the type it returns. We can't stop people from writing misleading docs without taking functionality away that would be useful in the right hands.

jsha · 2022-05-05T00:50:59Z

I think @notriddle's comment makes sense and I think the image @GuillaumeGomez showed is fine.

bors · 2022-05-05T05:50:56Z

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

GuillaumeGomez · 2022-05-05T08:50:20Z

I said I was fine with it but thanks for the extra explanations. :) r=me once you fixed the conflict.

Co-Authored-By: Guillaume Gomez <guillaume1.gomez@gmail.com>

* If it's just `-> a`, use "In Function Return Types" * If it's just `a b`, use "In Function Parameters" * Otherwise, still use "In Function Signatures"

notriddle · 2022-05-05T16:40:48Z

@bors r=GuillaumeGomez,jsha

bors · 2022-05-05T16:40:50Z

📌 Commit 4c183cd has been approved by GuillaumeGomez,jsha

…ch, r=GuillaumeGomez,jsha rustdoc: when running a function-signature search, tweak the tab bar # Before ![In Names (7) / In Parameters (0) / In Return types (0)](https://user-images.githubusercontent.com/1593513/166122875-ffdeafe6-8d4d-4e61-84a6-f5986b50ac35.png) # After ![In Function Signature (7)](https://user-images.githubusercontent.com/1593513/166122883-9a3d7515-3235-4ee3-8c4b-5401d109e099.png)

notriddle · 2022-05-06T15:19:51Z

r? @GuillaumeGomez

…ch, r=GuillaumeGomez,jsha rustdoc: when running a function-signature search, tweak the tab bar # Before ![In Names (7) / In Parameters (0) / In Return types (0)](https://user-images.githubusercontent.com/1593513/166122875-ffdeafe6-8d4d-4e61-84a6-f5986b50ac35.png) # After ![In Function Signature (7)](https://user-images.githubusercontent.com/1593513/166122883-9a3d7515-3235-4ee3-8c4b-5401d109e099.png)

…laumeGomez Rollup of 10 pull requests Successful merges: - rust-lang#96557 (Allow inline consts to reference generic params) - rust-lang#96590 (rustdoc: when running a function-signature search, tweak the tab bar) - rust-lang#96650 (Collect function instance used in `global_asm!` sym operand) - rust-lang#96733 (turn `append_place_to_string` from recursion into iteration) - rust-lang#96748 (Fixes reexports in search) - rust-lang#96752 (Put the incompatible_closure_captures lint messages in alphabetical order) - rust-lang#96754 (rustdoc: ensure HTML/JS side implementors don't have dups) - rust-lang#96772 (Suggest fully qualified path with appropriate params) - rust-lang#96776 (Fix two minor issues in hir.rs) - rust-lang#96782 (a small `mirror_expr` cleanup) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned CraftSpider Apr 30, 2022

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 30, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2022

GuillaumeGomez reviewed May 1, 2022

View reviewed changes

src/test/rustdoc-gui/search-tab-change-title-fn-sig.goml Outdated Show resolved Hide resolved

GuillaumeGomez reviewed May 1, 2022

View reviewed changes

src/librustdoc/html/static/js/search.js Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

notriddle force-pushed the notriddle/tab-bar-fn-search branch from 1a4840f to 88aabf8 Compare May 2, 2022 23:18

GuillaumeGomez reviewed May 3, 2022

View reviewed changes

src/librustdoc/html/static/js/search.js Outdated Show resolved Hide resolved

GuillaumeGomez reviewed May 3, 2022

View reviewed changes

src/test/rustdoc-gui/search-tab-change-title-fn-sig.goml Outdated Show resolved Hide resolved

notriddle and others added 5 commits May 5, 2022 09:37

rustdoc: when running a function-signature search, tweak the tab bar

6c8a2d4

Use STARTS_WITH, since it's more specific

345a580

Co-Authored-By: Guillaume Gomez <guillaume1.gomez@gmail.com>

rustdoc: change the "In Function Signatures" to context-sensitive

21a1213

* If it's just `-> a`, use "In Function Return Types" * If it's just `a b`, use "In Function Parameters" * Otherwise, still use "In Function Signatures"

rustdoc: fix keyboard shortcuts and console log on search page

8b2147b

rustdoc: add test case assertions for ArrowDown highlight first result

75790fa

rustdoc: fix JS error when rendering parse error

4c183cd

notriddle force-pushed the notriddle/tab-bar-fn-search branch from 45cf0be to 4c183cd Compare May 5, 2022 16:40

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2022

compiler-errors mentioned this pull request May 5, 2022

Rollup of 8 pull requests #96749

Closed

compiler-errors mentioned this pull request May 6, 2022

Rollup of 8 pull requests #96753

Closed

rust-highfive assigned GuillaumeGomez and unassigned CraftSpider May 6, 2022

GuillaumeGomez mentioned this pull request May 6, 2022

Rollup of 10 pull requests #96774

Closed

GuillaumeGomez mentioned this pull request May 6, 2022

Rollup of 10 pull requests #96775

Closed

GuillaumeGomez mentioned this pull request May 6, 2022

Rollup of 10 pull requests #96785

Merged

bors merged commit fcb0bce into rust-lang:master May 6, 2022

rustbot added this to the 1.62.0 milestone May 6, 2022

Uh oh!

Conversation

notriddle commented Apr 30, 2022

Before

After

Uh oh!

rust-highfive commented Apr 30, 2022

Uh oh!

rust-highfive commented Apr 30, 2022

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented May 2, 2022

Uh oh!

Manishearth commented May 2, 2022

Uh oh!

jsha commented May 2, 2022

Uh oh!

GuillaumeGomez commented May 2, 2022

Uh oh!

GuillaumeGomez commented May 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

notriddle commented May 3, 2022

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented May 3, 2022

Uh oh!

GuillaumeGomez commented May 3, 2022

Uh oh!

notriddle commented May 3, 2022

Uh oh!

GuillaumeGomez commented May 3, 2022

Uh oh!

notriddle commented May 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsha commented May 5, 2022

Uh oh!

bors commented May 5, 2022

Uh oh!

GuillaumeGomez commented May 5, 2022

Uh oh!

notriddle commented May 5, 2022

Uh oh!

bors commented May 5, 2022

Uh oh!

notriddle commented May 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

GuillaumeGomez commented May 2, 2022 •

edited

Loading

notriddle commented May 5, 2022 •

edited

Loading