rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
95.17k stars 12.27k forks source link

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

Open camelid opened 2 years ago

camelid commented 2 years ago

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

alexcrichton commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

willcrichton commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

PicoJr commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago
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:** ```toml # Cargo.toml [package] name = "mycrate" version = "0.1.0" edition = "2021" [dependencies] rand = "0.8" ``` ```rust //! /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; } ``` ```rust //! /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::()); } #[cfg(test)] mod test { #[test] fn testing_world() { assert![true]; } } ``` **testing it out:** ```sh $ 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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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:

camelid commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

I likes me some option 3

jsha commented 2 years ago

With https://github.com/rust-lang/rust/issues/59829, https://github.com/rust-lang/rust/issues/59851#issuecomment-488976397, 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 commented 2 years ago

@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.

willcrichton commented 2 years ago

After a lot of discussion on Zulip and some implementation work, most of the remaining issues are being addressed in these PRs:

Emilgardis commented 1 year ago

On a note related to https://github.com/rust-lang/rust/issues/88791#issuecomment-963080815

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 commented 7 months ago

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

willcrichton commented 6 months ago

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:

https://github.com/rust-lang/rust/blob/e87ccb8676be9ab641849a2539b215d0c9027237/src/librustdoc/scrape_examples.rs#L174-L176

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 commented 6 months ago

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

OliverKillane commented 3 months ago

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