Skip to content

Commit

Permalink
Rollup merge of #105387 - willcrichton:scrape-examples-ui-improvement…
Browse files Browse the repository at this point in the history
…s, 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!
  • Loading branch information
matthiaskrgr committed Dec 9, 2022
2 parents 0b4d57b + 9499d2c commit 5fd540b
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 25 deletions.
5 changes: 5 additions & 0 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ pub(crate) struct Options {
pub(crate) input: PathBuf,
/// The name of the crate being documented.
pub(crate) crate_name: Option<String>,
/// Whether or not this is a bin crate
pub(crate) bin_crate: bool,
/// Whether or not this is a proc-macro crate
pub(crate) proc_macro_crate: bool,
/// How to format errors and warnings.
Expand Down Expand Up @@ -176,6 +178,7 @@ impl fmt::Debug for Options {
f.debug_struct("Options")
.field("input", &self.input)
.field("crate_name", &self.crate_name)
.field("bin_crate", &self.bin_crate)
.field("proc_macro_crate", &self.proc_macro_crate)
.field("error_format", &self.error_format)
.field("libs", &self.libs)
Expand Down Expand Up @@ -667,6 +670,7 @@ impl Options {
None => OutputFormat::default(),
};
let crate_name = matches.opt_str("crate-name");
let bin_crate = crate_types.contains(&CrateType::Executable);
let proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
let playground_url = matches.opt_str("playground-url");
let maybe_sysroot = matches.opt_str("sysroot").map(PathBuf::from);
Expand Down Expand Up @@ -718,6 +722,7 @@ impl Options {
rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref());
let options = Options {
input,
bin_crate,
proc_macro_crate,
error_format,
diagnostic_width,
Expand Down
19 changes: 14 additions & 5 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2957,14 +2957,23 @@ fn render_call_locations(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Ite

// The call locations are output in sequence, so that sequence needs to be determined.
// Ideally the most "relevant" examples would be shown first, but there's no general algorithm
// for determining relevance. Instead, we prefer the smallest examples being likely the easiest to
// understand at a glance.
// for determining relevance. We instead proxy relevance with the following heuristics:
// 1. Code written to be an example is better than code not written to be an example, e.g.
// a snippet from examples/foo.rs is better than src/lib.rs. We don't know the Cargo
// directory structure in Rustdoc, so we proxy this by prioritizing code that comes from
// a --crate-type bin.
// 2. Smaller examples are better than large examples. So we prioritize snippets that have
// the smallest number of lines in their enclosing item.
// 3. Finally we sort by the displayed file name, which is arbitrary but prevents the
// ordering of examples from randomly changing between Rustdoc invocations.
let ordered_locations = {
let sort_criterion = |(_, call_data): &(_, &CallData)| {
fn sort_criterion<'a>(
(_, call_data): &(&PathBuf, &'a CallData),
) -> (bool, u32, &'a String) {
// Use the first location because that's what the user will see initially
let (lo, hi) = call_data.locations[0].enclosing_item.byte_span;
hi - lo
};
(!call_data.is_bin, hi - lo, &call_data.display_name)
}

let mut locs = call_locations.iter().collect::<Vec<_>>();
locs.sort_by_key(sort_criterion);
Expand Down
37 changes: 35 additions & 2 deletions src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -1813,6 +1813,22 @@ in storage.js
}
}

/* Should have min-width: (N + 1)px where N is the mobile breakpoint above. */
@media (min-width: 701px) {
/* Places file-link for a scraped example on top of the example to save space.
We only do this on large screens so the file-link doesn't overlap too much
with the example's content. */
.scraped-example-title {
position: absolute;
z-index: 10;
background: var(--main-background-color);
bottom: 8px;
right: 5px;
padding: 2px 4px;
box-shadow: 0 0 4px var(--main-background-color);
}
}

@media print {
nav.sidebar, nav.sub, .out-of-band, a.srclink, #copy-path,
details.rustdoc-toggle[open] > summary::before, details.rustdoc-toggle > summary::before,
Expand Down Expand Up @@ -1899,6 +1915,11 @@ in storage.js
border-radius: 50px;
}

.scraped-example {
/* So .scraped-example-title can be positioned absolutely */
position: relative;
}

.scraped-example .code-wrapper {
position: relative;
display: flex;
Expand All @@ -1908,18 +1929,30 @@ in storage.js
}

.scraped-example:not(.expanded) .code-wrapper {
max-height: 240px;
/* scrape-examples.js has a constant DEFAULT_MAX_LINES (call it N) for the number
* of lines shown in the un-expanded example code viewer. This pre needs to have
* a max-height equal to line-height * N. The line-height is currently 1.5em,
* and we include additional 10px for padding. */
max-height: calc(1.5em * 5 + 10px);
}

.scraped-example:not(.expanded) .code-wrapper pre {
overflow-y: hidden;
max-height: 240px;
padding-bottom: 0;
/* See above comment, should be the same max-height. */
max-height: calc(1.5em * 5 + 10px);
}

.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper,
.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper pre {
/* See above comment, except this height is based on HIDDEN_MAX_LINES. */
max-height: calc(1.5em * 10 + 10px);
}

.scraped-example .code-wrapper .next,
.scraped-example .code-wrapper .prev,
.scraped-example .code-wrapper .expand {
color: var(--main-color);
position: absolute;
top: 0.25em;
z-index: 1;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/static/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ function loadCss(cssUrl) {
const innerToggle = document.getElementById(toggleAllDocsId);
removeClass(innerToggle, "will-expand");
onEachLazy(document.getElementsByClassName("rustdoc-toggle"), e => {
if (!hasClass(e, "type-contents-toggle")) {
if (!hasClass(e, "type-contents-toggle") && !hasClass(e, "more-examples-toggle")) {
e.open = true;
}
});
Expand Down
32 changes: 20 additions & 12 deletions src/librustdoc/html/static/js/scrape-examples.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,41 @@
"use strict";

(function() {
// Number of lines shown when code viewer is not expanded
const MAX_LINES = 10;
// Number of lines shown when code viewer is not expanded.
// DEFAULT is the first example shown by default, while HIDDEN is
// the examples hidden beneath the "More examples" toggle.
//
// NOTE: these values MUST be synchronized with certain rules in rustdoc.css!
const DEFAULT_MAX_LINES = 5;
const HIDDEN_MAX_LINES = 10;

// Scroll code block to the given code location
function scrollToLoc(elt, loc) {
function scrollToLoc(elt, loc, isHidden) {
const lines = elt.querySelector(".src-line-numbers");
let scrollOffset;

// If the block is greater than the size of the viewer,
// then scroll to the top of the block. Otherwise scroll
// to the middle of the block.
if (loc[1] - loc[0] > MAX_LINES) {
const maxLines = isHidden ? HIDDEN_MAX_LINES : DEFAULT_MAX_LINES;
if (loc[1] - loc[0] > maxLines) {
const line = Math.max(0, loc[0] - 1);
scrollOffset = lines.children[line].offsetTop;
} else {
const wrapper = elt.querySelector(".code-wrapper");
const halfHeight = wrapper.offsetHeight / 2;
const offsetMid = (lines.children[loc[0]].offsetTop
+ lines.children[loc[1]].offsetTop) / 2;
const offsetTop = lines.children[loc[0]].offsetTop;
const lastLine = lines.children[loc[1]];
const offsetBot = lastLine.offsetTop + lastLine.offsetHeight;
const offsetMid = (offsetTop + offsetBot) / 2;
scrollOffset = offsetMid - halfHeight;
}

lines.scrollTo(0, scrollOffset);
elt.querySelector(".rust").scrollTo(0, scrollOffset);
}

function updateScrapedExample(example) {
function updateScrapedExample(example, isHidden) {
const locs = JSON.parse(example.attributes.getNamedItem("data-locs").textContent);
let locIndex = 0;
const highlights = Array.prototype.slice.call(example.querySelectorAll(".highlight"));
Expand All @@ -40,7 +48,7 @@
const onChangeLoc = changeIndex => {
removeClass(highlights[locIndex], "focus");
changeIndex();
scrollToLoc(example, locs[locIndex][0]);
scrollToLoc(example, locs[locIndex][0], isHidden);
addClass(highlights[locIndex], "focus");

const url = locs[locIndex][1];
Expand Down Expand Up @@ -70,19 +78,19 @@
expandButton.addEventListener("click", () => {
if (hasClass(example, "expanded")) {
removeClass(example, "expanded");
scrollToLoc(example, locs[0][0]);
scrollToLoc(example, locs[0][0], isHidden);
} else {
addClass(example, "expanded");
}
});
}

// Start with the first example in view
scrollToLoc(example, locs[0][0]);
scrollToLoc(example, locs[0][0], isHidden);
}

const firstExamples = document.querySelectorAll(".scraped-example-list > .scraped-example");
onEachLazy(firstExamples, updateScrapedExample);
onEachLazy(firstExamples, el => updateScrapedExample(el, false));
onEachLazy(document.querySelectorAll(".more-examples-toggle"), toggle => {
// Allow users to click the left border of the <details> section to close it,
// since the section can be large and finding the [+] button is annoying.
Expand All @@ -99,7 +107,7 @@
// depends on offsetHeight, a property that requires an element to be visible to
// compute correctly.
setTimeout(() => {
onEachLazy(moreExamples, updateScrapedExample);
onEachLazy(moreExamples, el => updateScrapedExample(el, true));
});
}, {once: true});
});
Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/html/static/scrape-examples-help.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Rustdoc will automatically scrape examples of documented items from the `examples/` directory of a project. These examples will be included within the generated documentation for that item. For example, if your library contains a public function:
Rustdoc will automatically scrape examples of documented items from a project's source code. These examples will be included within the generated documentation for that item. For example, if your library contains a public function:

```rust
// src/lib.rs
Expand All @@ -16,6 +16,7 @@ fn main() {

Then this code snippet will be included in the documentation for `a_func`.


## How to read scraped examples

Scraped examples are shown as blocks of code from a given file. The relevant item will be highlighted. If the file is larger than a couple lines, only a small window will be shown which you can expand by clicking &varr; in the top-right. If a file contains multiple instances of an item, you can use the &pr; and &sc; buttons to toggle through each instance.
Expand All @@ -25,7 +26,7 @@ If there is more than one file that contains examples, then you should click "Mo

## How Rustdoc scrapes examples

When you run `cargo doc`, Rustdoc will analyze all the crates that match Cargo's `--examples` filter for instances of items that occur in the crates being documented. Then Rustdoc will include the source code of these instances in the generated documentation.
When you run `cargo doc -Zunstable-options -Zrustdoc-scrape-examples`, Rustdoc will analyze all the documented crates for uses of documented items. Then Rustdoc will include the source code of these instances in the generated documentation.

Rustdoc has a few techniques to ensure this doesn't overwhelm documentation readers, and that it doesn't blow up the page size:

Expand Down
10 changes: 9 additions & 1 deletion src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ fn main_args(at_args: &[String]) -> MainResult {
let output_format = options.output_format;
let externs = options.externs.clone();
let scrape_examples_options = options.scrape_examples_options.clone();
let bin_crate = options.bin_crate;

let config = core::create_config(options);

Expand Down Expand Up @@ -832,7 +833,14 @@ fn main_args(at_args: &[String]) -> MainResult {
info!("finished with rustc");

if let Some(options) = scrape_examples_options {
return scrape_examples::run(krate, render_opts, cache, tcx, options);
return scrape_examples::run(
krate,
render_opts,
cache,
tcx,
options,
bin_crate,
);
}

cache.crate_version = crate_version;
Expand Down
10 changes: 8 additions & 2 deletions src/librustdoc/scrape_examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ pub(crate) struct CallData {
pub(crate) url: String,
pub(crate) display_name: String,
pub(crate) edition: Edition,
pub(crate) is_bin: bool,
}

pub(crate) type FnCallLocations = FxHashMap<PathBuf, CallData>;
Expand All @@ -122,6 +123,7 @@ struct FindCalls<'a, 'tcx> {
cx: Context<'tcx>,
target_crates: Vec<CrateNum>,
calls: &'a mut AllCallLocations,
bin_crate: bool,
}

impl<'a, 'tcx> Visitor<'tcx> for FindCalls<'a, 'tcx>
Expand Down Expand Up @@ -245,7 +247,9 @@ where
let mk_call_data = || {
let display_name = file_path.display().to_string();
let edition = call_span.edition();
CallData { locations: Vec::new(), url, display_name, edition }
let is_bin = self.bin_crate;

CallData { locations: Vec::new(), url, display_name, edition, is_bin }
};

let fn_key = tcx.def_path_hash(*def_id);
Expand Down Expand Up @@ -274,6 +278,7 @@ pub(crate) fn run(
cache: formats::cache::Cache,
tcx: TyCtxt<'_>,
options: ScrapeExamplesOptions,
bin_crate: bool,
) -> interface::Result<()> {
let inner = move || -> Result<(), String> {
// Generates source files for examples
Expand All @@ -300,7 +305,8 @@ pub(crate) fn run(

// Run call-finder on all items
let mut calls = FxHashMap::default();
let mut finder = FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates };
let mut finder =
FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates, bin_crate };
tcx.hir().visit_all_item_likes_in_crate(&mut finder);

// The visitor might have found a type error, which we need to
Expand Down
2 changes: 2 additions & 0 deletions src/test/rustdoc-gui/scrape-examples-button-focus.goml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
goto: "file://" + |DOC_PATH| + "/scrape_examples/fn.test.html"

// The next/prev buttons vertically scroll the code viewport between examples
store-property: (initialScrollTop, ".scraped-example-list > .scraped-example pre", "scrollTop")
focus: ".scraped-example-list > .scraped-example .next"
press-key: "Enter"
Expand All @@ -12,6 +13,7 @@ assert-property: (".scraped-example-list > .scraped-example pre", {
"scrollTop": |initialScrollTop|
})

// The expand button increases the scrollHeight of the minimized code viewport
store-property: (smallOffsetHeight, ".scraped-example-list > .scraped-example pre", "offsetHeight")
assert-property-false: (".scraped-example-list > .scraped-example pre", {
"scrollHeight": |smallOffsetHeight|
Expand Down
14 changes: 14 additions & 0 deletions src/test/rustdoc-gui/scrape-examples-toggle.goml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
goto: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html"

// Clicking "More examples..." will open additional examples
assert-attribute-false: (".more-examples-toggle", {"open": ""})
click: ".more-examples-toggle"
assert-attribute: (".more-examples-toggle", {"open": ""})

// Toggling all docs will close additional examples
click: "#toggle-all-docs"
assert-attribute-false: (".more-examples-toggle", {"open": ""})

// After re-opening the docs, the additional examples should stay closed
click: "#toggle-all-docs"
assert-attribute-false: (".more-examples-toggle", {"open": ""})

0 comments on commit 5fd540b

Please sign in to comment.