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.

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

Improve Rustdoc scrape-examples UI #105387

Merged
merged 10 commits into from
Dec 9, 2022

Conversation

willcrichton
Copy link
Contributor

@willcrichton willcrichton commented Dec 6, 2022

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

1. The first scraped example now takes up significantly less screen height.

Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned over the example instead of atop the example (only on desktop though, not mobile). The changes to scrape-examples.js and rustdoc.css implement this fix.

Here is what an example docblock now looks like:

Screen Shot 2022-12-06 at 10 02 21 AM

2. Expanding all docblocks will not expand "More examples".

The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to main.js implement this fix. This is tested in scrape-examples-toggle.goml.

3. Examples from binary crates are sorted higher than examples from library crates.

Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from --crate-type.

This change is implemented by adding a new field bin_crate in Options (see config.rs). An is_bin field has been added to the scraped examples metadata (see scrape_examples.rs). Then the example sorting metric uses is_bin as the first entry of a lexicographic sort on (is_bin, example_size, display_name) (see render/mod.rs).

Note that in the future we can consider adding another flag like --scrape-examples-cargo-target that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See scrape-examples-help.md.

r? @notriddle

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!

@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 Dec 6, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2022

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@notriddle
Copy link
Contributor

Problem I noticed (though it's also a problem in current nightly).

image

Dark theme needs some work on the next/prev/expand buttons.

@rust-log-analyzer

This comment has been minimized.

@willcrichton
Copy link
Contributor Author

@notriddle good catch, just fixed the button colors in the latest commit.

@willcrichton willcrichton force-pushed the scrape-examples-ui-improvements branch 2 times, most recently from 96e0ff9 to aff1436 Compare December 6, 2022 19:27
padding-bottom: 0;
}

.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper,
.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper pre {
max-height: 240px;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this is synchronized with the _LINES constants in scrape-examples.js.

It should definitely have a comment pointing this out, but it also probably shouldn’t be set in px. Someone might adjust their font size.

@GuillaumeGomez
Copy link
Member

Apart from @notriddle's comment, looks good to me.

@willcrichton willcrichton force-pushed the scrape-examples-ui-improvements branch from fbb3d64 to 5319503 Compare December 7, 2022 18:25
@willcrichton
Copy link
Contributor Author

willcrichton commented Dec 7, 2022

@notriddle I updated the calculation of the viewer height to use em instead of px (see 9499d2c), along with some comments noting the JS/CSS link.

I also fixed a small issue where the midpoint of the highlighted line was being calculated with respect to a slightly wrong bottom position.

@willcrichton willcrichton force-pushed the scrape-examples-ui-improvements branch from 5319503 to 9499d2c Compare December 7, 2022 18:42
@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 7, 2022

📌 Commit 9499d2c has been approved by notriddle

It is now in the queue for this repository.

@bors 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 Dec 7, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2022
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? `@notriddle`

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2022
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? ``@notriddle``

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
@matthiaskrgr
Copy link
Member

I'm wondering if this failed in #105448 (comment) since it's the only rustdoc change but I am not sure.

@willcrichton
Copy link
Contributor Author

willcrichton commented Dec 8, 2022

@matthiaskrgr I just checked that the failing test does pass on the scrape-examples-ui-improvements when I run locally, so I'm not sure why it would fail in rollup.

CI is failing here: https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L125

So cx.output_base_dir() is an "invalid input" to create_dir_all for some reason?

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2022
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#105216 (Remove unused GUI test)
 - rust-lang#105245 (attempt to clarify align_to docs)
 - rust-lang#105387 (Improve Rustdoc scrape-examples UI)
 - rust-lang#105389 (Enable profiler in dist-powerpc64le-linux)
 - rust-lang#105427 (Dont silently ignore rustdoc errors)
 - rust-lang#105442 (rustdoc: clean up docblock table CSS)
 - rust-lang#105443 (Move some queries and methods)
 - rust-lang#105455 (use the correct `Reveal` during validation)
 - rust-lang#105470 (Clippy: backport ICE fix before beta branch)
 - rust-lang#105474 (lib docs: fix typo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5fd540b into rust-lang:master Dec 9, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 9, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? `@notriddle`

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#105216 (Remove unused GUI test)
 - rust-lang#105245 (attempt to clarify align_to docs)
 - rust-lang#105387 (Improve Rustdoc scrape-examples UI)
 - rust-lang#105389 (Enable profiler in dist-powerpc64le-linux)
 - rust-lang#105427 (Dont silently ignore rustdoc errors)
 - rust-lang#105442 (rustdoc: clean up docblock table CSS)
 - rust-lang#105443 (Move some queries and methods)
 - rust-lang#105455 (use the correct `Reveal` during validation)
 - rust-lang#105470 (Clippy: backport ICE fix before beta branch)
 - rust-lang#105474 (lib docs: fix typo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants