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

Tracking Issue for RFC 3123: Scrape code examples from examples/ directory for Rustdoc #88791

Open
2 of 4 tasks
camelid opened this issue Sep 9, 2021 · 43 comments
Open
2 of 4 tasks
Labels
A-rustdoc-scrape-examples Area: The (unstable) rustdoc scrape-examples feature described in RFC 3123 B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Sep 9, 2021

This is a tracking issue for the RFC "Scrape code examples from examples/ directory for Rustdoc" (rust-lang/rfcs#3123).

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  1. Is showing 1 example by default the best idea? Or should scraped examples be hidden by default?
  2. Is the ability to see the full file context worth the increase in page size?
  3. How should the examples be ordered? Is there a way to determine the "best" examples to show first?

Implementation history

@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Sep 9, 2021
@alexcrichton
Copy link
Member

I left a comment on the Cargo PR but to summarize here, I don't think that the current cargo doc --scrape-examples interface (requesting this via the CLI) is the best way to stabilize an interface into this through Cargo. Instead I think that it would be better to support this configuration via the manifest where targets (e.g. libraries, binaries, etc) can each individually be configured whether or not they are considered candidates for scraping examples. That will change part of the Cargo implementation (but not all) in a way that I don't expect to be too radical but probably still a chunk of work to get done before final stabilization.

@camelid
Copy link
Member Author

camelid commented Oct 26, 2021

That will change part of the Cargo implementation (but not all) in a way that I don't expect to be too radical but probably still a chunk of work to get done before final stabilization.

Yeah, we'll still want to test this feature for a while before stabilizing, so we have time :)

@vicky5124
Copy link

Some crates make rustdoc panic with this feature being used, a couple of these are:

serenity, which leads to these logs: https://pastebin.com/smwdrWbD
Using a relatively standard cargo doc command:
RUST_BACKTRACE=full RUSTDOCFLAGS="--cfg docsrs -D rustdoc::broken_intra_doc_links" cargo +nightly doc -Z unstable-options -Z rustdoc-scrape-examples=all --open --no-deps --all-features

A similar issue happens with lavalink-rs, where the serenity example panics rustdoc with these logs: https://pastebin.com/sHJ0YZh6
But if it's removed from the workspaces on Cargo.toml, it compiles with the twilight example, but with an issue: none of the methods used in the example show up in the docs.

To show that this is not an issue strictly with serenity, poise, a library which depends on serenity, uses it on its examples, and re-exports it, does not have this issue, as show by it having its docs built this way.

Note: rustdoc-scrape-examples=all is used instead of rustdoc-scrape-examples=examples because both of the libraries with issues use subcrates as examples; I thought this was the reason as to why the panic happens, but a minimal crate with the same kind of examples worked just fine. I also tried to see if proc-macros were causing the panic, but that's not the case either.

@camelid camelid added the A-rustdoc-scrape-examples Area: The (unstable) rustdoc scrape-examples feature described in RFC 3123 label Nov 4, 2021
@willcrichton
Copy link
Contributor

@vicky5124 thanks for the bug reports. I have fixed these issues (macro-related) in #90583 which should hopefully land by the time today's nightly (2021-11-05) is released.

@programatik29
Copy link

I did a quick test.

Scraping examples with this Cargo.toml does not work:

[package]
name = "scrape-me"
version = "0.1.0"
edition = "2021"

[dependencies]

This works:

[package]
name = "scrape_me" # '_' instead of '-'
version = "0.1.0"
edition = "2021"

[dependencies]

@willcrichton
Copy link
Contributor

@programatik29 thanks for the report. This is issue rust-lang/cargo#10035 which has been fixed in rust-lang/cargo#10037. We just have to wait for the cargo submodule in rustc to get updated (cc @ehuss).

@programatik29
Copy link

Nice. This feature is amazing. Can't wait to use it in my docs.

@PicoJr
Copy link

PicoJr commented Nov 8, 2021

Building doc locally works fine

cargo +nightly doc -Z unstable-options -Z rustdoc-scrape-examples=examples

does pick up the right code snippet from my examples/ folder for my crate htp.

It looks great btw.

Online rustdoc (does not seem to work with my crate)

I have not managed to have my crate's doc benefit from this new feature, here is my Cargo.toml:

https://github.com/PicoJr/htp/blob/6faea37ad8f1c63a9992d16391389fda5d79011d/Cargo.toml#L21-L23

I use the section described here.

(edit) Could it be because of the include section of my Cargo.toml? here are the doc build logs for my crate: https://docs.rs/crate/htp/0.4.1/builds/459893

@willcrichton
Copy link
Contributor

willcrichton commented Nov 8, 2021

@PicoJr yes, it is your include section. docs.rs will download the published version of your crate, and if the examples aren't included, then they can't be scraped.

Note to everyone reading this: if you see the following warning in your cargo doc logs, then your examples are either misconfigured or not published:

Warning: Target filter `examples` specified, but no targets matched. This is a no-op

@PicoJr
Copy link

PicoJr commented Nov 8, 2021

@PicoJr yes, it is your include section. docs.rs will download the published version of your crate, and if the examples aren't included, then they can't be scraped.

Note to everyone reading this: if you see the following warning in your cargo doc logs, then your examples are either misconfigured or not published:

Warning: Target filter `examples` specified, but no targets matched. This is a no-op

@willcrichton thanks for your reply it does work now.

@joseluis
Copy link
Contributor

joseluis commented Dec 5, 2021

Hi, I just found this RFC and funnily enough I recently devised a way to achieve this in current stable. So I wanted to share my solution. The following minimal project shows how to include documentation and tests from the examples directory:

project files:

# Cargo.toml
[package]
name = "mycrate"
version = "0.1.0"
edition = "2021"

[dependencies]
rand = "0.8"
//! /src/lib.rs

/// Hello from lib.
#[derive(Debug)]
pub struct Hello;

#[path = "../examples"]
#[cfg(any(doc, test))]
pub mod examples {
    #![allow(dead_code)]

    pub mod world;
}
//! /examples/world.rs

// external dependencies:
#[cfg(not(doc))]
use rand::random;

// internal dependencies:
#[cfg(any(test, doc))]
use crate::Hello;
#[cfg(not(any(test, doc)))]
use mycrate::Hello;

/// World example.
#[derive(Debug)]
pub struct World;

fn main() {
    let (h, w) = (Hello, World);
    println!("{:?} {:?} {}", h, w, random::<u8>());
}

#[cfg(test)]
mod test {
    #[test]
    fn testing_world() {
        assert![true];
    }
}

testing it out:

$ cargo doc --open # shows the example documentation ok

$ cargo run --quiet --example world # runs the example ok
Hello World 159

$ cargo test testing_world # runs the example test ok

running 1 test
test examples::world::test::testing_world ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

EDIT: @camelid you're right, my mistake

@camelid
Copy link
Member Author

camelid commented Dec 5, 2021

@joseluis That's different from this feature. This feature is about showing snippets of examples in the documentation for functions that are called from the examples.

@willcrichton
Copy link
Contributor

The feature has now been on nightly for about two months. I've been able to fix all the bug reports raised thus far, and I have not received any further suggestions on the UI design. Are there any objections to moving forward with stabilization?

Note that we still need to separately stabilize the Cargo changes (rust-lang/cargo#9910).

cc @jyn514 @camelid @GuillaumeGomez

@camelid
Copy link
Member Author

camelid commented Dec 22, 2021

I have not actually seen it in use yet. I'd like to see it in practice a bit before stabilizing. Do you know of some crates that enable it on docs.rs?

@PicoJr
Copy link

PicoJr commented Dec 22, 2021

I have not actually seen it in use yet. I'd like to see it in practice a bit before stabilizing. Do you know of some crates that enable it on docs.rs?

I enabled it for my crate htp, here is how it looks on doc.rs.

Is there a tool for searching crates that depend on this feature?

@jyn514
Copy link
Member

jyn514 commented Dec 22, 2021

You could download the source for all crates.io crates and search for rustdoc-scrape-examples. There's instructions on https://www.pietroalbini.org/blog/downloading-crates-io/, I think https://github.com/the-lean-crate/criner/tree/main/criner automates it.

@GuillaumeGomez
Copy link
Member

@willcrichton I wanted to try to improve the JS side to reduce its size but it's not blocking for a stabilization I think.

@jyn514
Copy link
Member

jyn514 commented Jan 8, 2022

@Mark-Simulacrum a while ago suggested using --emit example-metadata or something like that (or maybe you were just suggesting that this would be part of normal .rlibs? I'm not sure how that works with the whole reverse-dependency thing though ...). We probably should iron that out before stabilizing.

https://zulip-archive.rust-lang.org/stream/266220-rustdoc/topic/--scrape-examples.20feedback.20.2388791.html

@willcrichton
Copy link
Contributor

willcrichton commented Jan 21, 2022

I followed @jyn514's advice and used criner to find crates with scrape-examples. Here are a few crates using this feature:

From others in the thread:

@programatik29
Copy link

@willcrichton I first tested this feature in scrape_me crate. There was a problem with - character that is now fixed.

I am currently using it in axum-server crate.

@willcrichton
Copy link
Contributor

Some thoughts as I'm looking through the crates above.

Highlighting large regions

Some examples, especially higher-order functions, correspond to large regions of code. For instance:

Screen Shot 2022-01-21 at 4 57 55 PM

This is kind of ugly, and also hard to read. One possibility might be to just highlight the function identifier, and then emphasize the function arguments in a different way?

Minimizing examples

Given that scraped examples take up a lot of screen real-estate, there should probably be a way of disabling them. Right now you can hit the global [-] button. But perhaps there should be another option to just minimize examples?

Optional warning for functions with no examples

It might be interesting to library authors to identify which functions have no scraped examples. That can serve as a signal that they might want to add examples involving a particular function.

@jsha
Copy link
Contributor

jsha commented Jan 22, 2022

Given that scraped examples take up a lot of screen real-estate, there should probably be a way of disabling them. Right now you can hit the global [-] button. But perhaps there should be another option to just minimize examples?

I'd like to avoid adding a new setting for minimizing examples separately from their containing item. I think the global [-] button does the job well enough.

Here's how I think of it: rustdoc is ultimately a tool of the documentation author to write the best, most useful, most readable documentation they can. It makes that job a lot easier, including organizing by type, extracting doccomments, and scraping examples. If a documentation author finds that their scraped examples take up too much room in their docs, they can arrange for those examples not to get scraped.

@camelid
Copy link
Member Author

camelid commented Jan 22, 2022

This is kind of ugly, and also hard to read. One possibility might be to just highlight the function identifier, and then emphasize the function arguments in a different way?

One possible solution is to elide the interior lines if there're more than e.g. 10. Also, highlighting the arguments in a different way sounds promising.

Given that scraped examples take up a lot of screen real-estate, there should probably be a way of disabling them. Right now you can hit the global [-] button. But perhaps there should be another option to just minimize examples?

I'm also opposed to adding reader configuration (i.e., settings that the end-user of the docs applies) since rustdoc already has tons of configuration. I think @jsha's explanation is a very good way of thinking about it.

It might be interesting to library authors to identify which functions have no scraped examples. That can serve as a signal that they might want to add examples involving a particular function.

Yeah, we could consider a lint for that, but it'd probably be best to save it for later, especially because we'd probably need to make example scraping the default first.

@GuillaumeGomez
Copy link
Member

One possible solution is to elide the interior lines if there're more than e.g. 10.

I agree. We can add ... after it and that should solve most problems with it.

Yeah, we could consider a lint for that, but it'd probably be best to save it for later, especially because we'd probably need to make example scraping the default first.

For later: agreed. I'm curious though: should we put it in rustdoc or in clippy?

@willcrichton
Copy link
Contributor

willcrichton commented Jan 22, 2022

One simple solution is to strip leading whitespace from the highlighted regions. There's still a lot of yellow, but it's better.

Screen Shot 2022-01-22 at 3 10 59 PM

I played around with different ways to highlight arguments distinct (or not at all) from the function call identifier. But nothing really felt both appealing and consistent across the different scenarios (1-line vs multi-line). Also there's already two different highlight colors when there are multiple call-sites in a single item, so introducing a third color felt too much.

See #93217 for the patch.

@camelid
Copy link
Member Author

camelid commented Jan 23, 2022

One simple solution is to strip leading whitespace from the highlighted regions. There's still a lot of yellow, but it's better.

Hmm, but what about multiline strings?

I played around with different ways to highlight arguments distinct (or not at all) from the function call identifier. But nothing really felt both appealing and consistent across the different scenarios (1-line vs multi-line). Also there's already two different highlight colors when there are multiple call-sites in a single item, so introducing a third color felt too much.

What about just highlighting the function name (and maybe the ( and )) and showing the arguments without highlighting?

@willcrichton
Copy link
Contributor

True, this doesn't handle multiline strings.

And I tried not highlighting arguments, but for single-line call sites, it looked weird. If I saw foo.bar("baz") it seemed more natural for ("baz") to be highlighted, since it is part of the example function call. And it would be strange to have different highlighting behavior for single-line and multi-line call sites.

@jsha
Copy link
Contributor

jsha commented Jan 23, 2022

I like the idea of just highlighting the item name. I'd actually like the scraped examples to use the same visual language as the non-scraped examples. Consistency helps people learn.

Proposal: In all documentation examples, we should emphasize the name of the item being documented. Perhaps by bolding it?

Also, if we want to apply highlighting to the line numbers, we should avoid yellow - which is used to indicate the linked-to line numbers in source view. Maybe bold the line numbers as well, to match the bolded item name?

@runiq
Copy link

runiq commented Jan 23, 2022

What about just highlighting the function name (and maybe the ( and )) and showing the arguments without highlighting?

I think I'd prefer this. Some additional options that could maybe help:

  • Highlight the arguments with a very low opacity setting.
  • Highlight the line numbers additionally.

@camelid
Copy link
Member Author

camelid commented Jan 23, 2022

Proposal: In all documentation examples, we should emphasize the name of the item being documented. Perhaps by bolding it?

Hmm, it's an interesting idea, although I'm not sure the extra implementation complexity is worth it. Scraped examples are handled by a totally different code path from non-scraped examples.

I also don't think we should use bold (or italics, for that matter) in code blocks. Bolded code doesn't look good IMO.

@willcrichton
Copy link
Contributor

willcrichton commented Jan 24, 2022

Here's what it would look like to just highlight the function name.

Screen Shot 2022-01-23 at 5 04 19 PM

Screen Shot 2022-01-23 at 5 04 42 PM

(Ignore the bug where method calls are including only the left parenthesis.)

Do y'all prefer this to highlighting the arguments?

@jsha
Copy link
Contributor

jsha commented Jan 24, 2022

I like that variant a lot!

Hmm, it's an interesting idea, although I'm not sure the extra implementation complexity is worth it. Scraped examples are handled by a totally different code path from non-scraped examples.

I feel its worth it. It shouldn't block implementation here, but it's worth doing eventually. As a reader, I look at the "Example" and "Examples found in repository" and wonder why the name is highlighted in one place and not the other.

I also don't think we should use bold (or italics, for that matter) in code blocks. Bolded code doesn't look good IMO.

I suspect that has a lot to do with the particular font-face and whether it has a dedicated bold weight or it is using fake bold. Source Code Pro has a nice variety of weights: https://fonts.google.com/specimen/Source+Code+Pro?query=source+code+pro. I'll try putting together an experiment at some point. The other option available to us is picking a specific syntax highlighting color that is always used for the item being documented.

@camelid
Copy link
Member Author

camelid commented Jan 24, 2022

I feel its worth it. It shouldn't block implementation here, but it's worth doing eventually. As a reader, I look at the "Example" and "Examples found in repository" and wonder why the name is highlighted in one place and not the other.

That's actually another UI concern of mine. I think we should try to make the scraped-examples section look different from user-written docs. The "Examples found in repository" appended as a regular Markdown heading has always bothered me a bit.

@willcrichton
Copy link
Contributor

@camelid here's a few concepts, like any of them?

Option 1: Left border

Screen Shot 2022-01-23 at 6 07 32 PM

Option 2: Top border

Screen Shot 2022-01-23 at 6 07 56 PM

Option 3: Box w/ inset shadow

Screen Shot 2022-01-23 at 6 06 33 PM

@tshepang
Copy link
Member

I likes me some option 3

@jsha
Copy link
Contributor

jsha commented Jan 24, 2022

With #59829, #59851 (comment), we're trying to reduce the use of horizontal lines spanning the page, because they draw attention more strongly than regular headings do, and can make it look like, e.g. "Examples found in repository" is a more important heading than pub fn arch. So of these 3, I prefer Option 1. Another possibility, if we need to strongly distinguish scraped examples, would be to show them with no background, as the mockup I linked above proposes. But I still don't understand the need to strongly distinguish scraped examples.

I think we should try to make the scraped-examples section look different from user-written docs. The "Examples found in repository" appended as a regular Markdown heading has always bothered me a bit.

@camelid can you explain why it's so important to distinguish the scraped examples strongly? From a reader perspective, the main thing is to understand that there is additional context and the example is not standalone. But I think that's adequately conveyed by the file name (and line number).

@willcrichton I appreciate your working through this! I know doing UI design via GitHub issue can be... time consuming. We can also start a Zulip thread tomorrow. That might make it a little faster to work out where we're each coming from. :-)

@camelid
Copy link
Member Author

camelid commented Jan 25, 2022

@camelid can you explain why it's so important to distinguish the scraped examples strongly? From a reader perspective, the main thing is to understand that there is additional context and the example is not standalone. But I think that's adequately conveyed by the file name (and line number).

I think just think it could be a little confusing or misleading for documentation readers and writers. For example, if someone sees an example that shouldn't show up (for whatever) reason, they may go look to edit the source, but get confused by not seeing the example there.

I'd also just rather keep a clear separation between user- and machine-generated content for consistency and clarity.


Of the options, I think I like Option 3 the best.

bors added a commit to rust-lang/crater that referenced this issue Apr 24, 2022
Add cargoflags to toolchain configuration

I am trying to test the new [scrape examples](rust-lang/rust#88791) feature using Crater. Because it's gated behind a Cargo flag `-Zrustdoc-scrape-examples`, I added the ability for Crater to take as input a new `+cargoflags=...` configuration for toolchains, similar to how `+rustflags` works.

One possible implementation issue: I'm doing `cargoflags.split(' ')` to split up the provided flags so as to pass them into the command builder. A more robust method would be to use a crate like [shlex](https://docs.rs/shlex/latest/shlex/). I'm not sure if this is important enough to add the dependency, or if there's a better way to do this.
@Emilgardis
Copy link
Contributor

Emilgardis commented Oct 23, 2022

On a note related to #88791 (comment)

it seems it's impossible to use rustdoc-scrape-examples=all and workspace members for examples on docs.rs, i.e the .crate tarball doesn't include the workspace members since they are not part of the published crate/package (not included in cargo package --list), and thus there is nothing to scrape. Adding the members in include doesn't solve it either.

My use-case is a crate with its examples being workspace members to include different dependencies without "polluting" dev-dependencies

@rcoh
Copy link
Contributor

rcoh commented Dec 14, 2023

It seems like this scrapes function usages but not macro usages. Is that something that's possible to support?

@willcrichton
Copy link
Contributor

It seems like this scrapes function usages but not macro usages. Is that something that's possible to support?

It would certainly be possible to support. There's a catch-all in scrape_examples.rs that could be filled in with more kinds of expressions:

_ => {
return;
}

There is a broader discussion about what kinds of items and what kinds of examples we want in Rustdoc. I focused on function calls for functions items, but there's nothing preventing us from scraping uses of constants, modules, and so on. @GuillaumeGomez @camelid do y'all have any thoughts about this?

@GuillaumeGomez
Copy link
Member

I think it'd make sense to also look for macros and scrape examples for them as well.

@OliverKillane
Copy link
Contributor

OliverKillane commented Apr 11, 2024

I think it'd make sense to also look for macros and scrape examples for them as well.

+1 for this. Would be very suave for documenting proc-macros, we currently have by default:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-scrape-examples Area: The (unstable) rustdoc scrape-examples feature described in RFC 3123 B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests