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

Code block continuation in documentation #2894

Closed
wants to merge 2 commits into from

Conversation

robinmoussu
Copy link

@robinmoussu robinmoussu commented Apr 1, 2020

Rendered EDIT: update rendered link

This is my first RFC, and I am still extremely new in rust world. I expect to have changes to make, and would welcome suggestion. I would like to implement it, but a bit of help (where to look mostly) would be welcome as well.

@kennytm kennytm added the T-rustdoc Relevant to rustdoc team, which will review and decide on the RFC. label Apr 1, 2020
@burdges
Copy link

burdges commented Apr 2, 2020

I've agree that setup for doc tests creates friction that reduces doc test quality, but we clearly /// should render like code comments, not even sure if this might be an 1st April joke.

Ideally one might close the code block, but then reopen it after some regular text.

Hello
```rust
```pause
World
```resume
```

@robinmoussu
Copy link
Author

pause and resume is a great idea. My issue with the alternative I proposed (continue_previous) was that it wasn't as flexible as I would like. The use of pause howevever allow for arbitrary things (even other code blocks?) in between. I think the interaction with cargo run is going to be more complicated, but this is probably a better direction.

@burdges
Copy link

burdges commented Apr 2, 2020

In principle you could even pause foo and resume foo multiple times in different orders with the text explaining relevance, and with the playground links loading everything, but not sure if humans can express that tangle or parse and disentangle it so easily.

@robinmoussu
Copy link
Author

I don't see the value of having the order or the code not following the reading order. We are talking about a documention, so something that shouldn't have too much complexity to be readable.


I can see the value of being to pause, and restart multiple time from the same point (like pausing after an initial set-up), but I think it is easy enough to just clone() the things that are mutated between each resume point to not bother with the extra complexity.

/// ```
/// let mut state = 0;
/// ``` pause 0
/// First use
/// ``` resume 0
/// foo(&mut state);
/// ```
/// Second use
/// ``` resume 0
/// bar(&mut state);
/// ```

Can be easily re-written as

/// ```
/// let set_up = 0;
/// ``` pause
/// First use
/// ``` resume
/// let mut state = set_up.clone();
/// foo(&mut state);
/// ```
/// Second use
/// ``` resume
/// let mut state = set_up.clone();
/// bar(&mut state);
/// ```

Even if the number of variable created in the set-up, I don't expect that the majority of them need to be mutated by each example. In the example that I gave with the graph, neither part of the set-up (once finished) had to be mutable.

@burdges
Copy link

burdges commented Apr 2, 2020

Agreed, you can even hide code block symbols to simulate those saved states, which looks more readable to rust devs, even if more verbose:

/// ```
/// let state = 0;
/// ``` pause
/// First use
/// ``` resume
/// # {
/// # let mut state = state.clone();
/// foo(&mut state);
/// # }
/// ```
/// Second use
/// ``` resume
/// # {
/// # let mut state = state.clone();
/// bar(&mut state);
/// # }
/// ```

@RustyYato
Copy link

Usually this is done by putting a macro in the docs that extracts out the common bits

```rust
# my_common_bits!();
let actual = example.here();
```

@robinmoussu
Copy link
Author

Usually this is done by putting a macro in the docs that extracts out the common bits

This approach have several drawback

  • If you want to display the content of the set-up, you need to duplicate it (one time in the macro, and one time in the doc).
  • You can't have more than one section.

For example if you want to explain how your state machine works, how to you express this?

//// Set-up
/// ```
/// let mut state = 0;
/// ``` pause
/// First transition
/// ``` resume
/// foo(&mut state);
/// ``` pause
/// Second transition
/// ``` resume
/// bar(&mut state);
/// ``` pause

@burdges
Copy link

burdges commented Apr 2, 2020

Are those objections really true? Would this work?

/// ```rust
/// # macro_rules! setup2 { () => { setup1!()
/// .. stuff ..
/// # } } setup2!()
/// ```

@robinmoussu
Copy link
Author

robinmoussu commented Apr 2, 2020

You will still write 2 times the content of the macro 2 times. Once in the macro, once in the doc. Witch is as bad as not creating the macro.

@joshtriplett
Copy link
Member

It seems worth taking some inspiration from "literate programming" concepts here, before independently reinventing our own.

@burdges
Copy link

burdges commented Apr 2, 2020

I exposed the internals of the macro in the .. stuff .. part, so no you only write it once.

I suspect the macro solution breaks the playground link though, well maybe only pause and resume make them work optimally.

@robinmoussu
Copy link
Author

I didn't know the name “literate programming”. I think that pause and continue gives exactly the same experience than a jupyter notebook. And it seems that jupyter notebooks are one way to do “literate programming”.

@Screwtapello
Copy link

If you're interested in Literate Programming and Rust, you may be interested in the Tango crate: https://github.com/pnkfelix/tango

@robinmoussu
Copy link
Author

@burdges I apologise, I did not understood what you said.

So I added

/// # Part 1
/// ``` rust
/// # macro_rules! p1 { () => {
/// let x = 1;
/// # } }
/// ```
/// # Part 2
/// ```
/// # p1!();
/// let y = x;
/// ```

cargo doc works (the documentation is correctly generated), but cargo test doesn't.

error: cannot find macro `p1` in this scope
  |
3 | p1!();
  | ^^

error[E0425]: cannot find value `x` in this scope
  |
4 | let y = x;
  |         ^ not found in this scope

For more information about this error, try `rustc --explain E0425`.

So it seems that it suffer exactly the same issue that a simple

/// ```
/// let x = 0;
/// ```
/// ```
/// let y = x;
/// ```

Whatever is declared in a given block isn't visible in the next one. So you need to create the macro outside of the documentation block, and thus repeat the content of the macro in the first code-block.

If I am wrong, please correct me.

@robinmoussu
Copy link
Author

robinmoussu commented Apr 3, 2020

To sum-up the current discussion:

  • my initial idea is garbage (I will eventually close this RFC and open a new one, but the discussion so far is really interesting).
  • the proposition of @burdges is amazing. It's the introduction of ```pause at the end of a rust code block and ``` continue a the beginning of a new one. cargo doc [documentation generators] can ignore it (just process them as normal rust code block, and everything in between as usual). If a [documention generator] generates a run button, the block starting by ``` continue will contains all the code in all the previous rust block with a ``` pause. cargo test should process a single snippet containing the aggregation of all those code-blocks. It is really simple to teach and to use. It seems to have no corner cases so far (except if someone pause and continue in the middle of a statement, but it's the responsibility of the documentation writer, and it all cases the user can use the last snippet witch is guaranteed to works). It compose really well with other features in documentation block (and I think that if the first block is no_run, should_panic, ignoreorcompile_fail`, then the whole aggregation should behave like it).
  • Macros don't seems a viable alternative.
  • As far as I can tell, this is exactly what “literate programming” is.

Do I have everything right?

@burdges
Copy link

burdges commented Apr 3, 2020

cargo doc should ideally generate playground links that merge all the pause and continue blocks, not ignore them.

If my macros do not work then how would the macros suggested by @KrishnaSannasi work? You put them in #[cfg(test)] mod tests { } modules inside the regular source?

@robinmoussu
Copy link
Author

cargo doc should ideally generate playground links that merge all the pause and continue blocks, not ignore them.

This is exactly what as said in the next phrase :) !

If a tool generates a run button, […]


If my macros do not work then how would the macros suggested by @KrishnaSannasi work? You put them in #[cfg(test)] mod tests { } modules inside the regular source?

As far as I know, yes. I didn't tested.

@jonhoo
Copy link
Contributor

jonhoo commented Apr 3, 2020

Hmm, how does the continue/pause mechanism interact with other fence arguments like no_run? Will the markdown parser get confused if you put keywords after the closing triple-backtick?

@robinmoussu
Copy link
Author

The html generation wouldn't change (except ignoring the keywords pause and continue). Links to the playground/any king of run buttons, cargo test would proccess the aggregation with the same fence as what was added to the first block. cargo test would only proccess the whole aggregate (not sub-aggregate).

@jonhoo
Copy link
Contributor

jonhoo commented Apr 3, 2020

Sorry, what I meant was that I don't know if the markdown parser that we currently use gets confused if the end-ticks for a code block are not exactly three backticks and nothing else on that line. If it got confused by that, that'd pose a problem.

As for no_run, ignore, and the like, it sounds then like your proposal is that the attributes from the code blocks are accumulated as you go? So if the first "part" has no_run, then all the parts will have no_run? What happens if the second "part" has no_run (but not the first)?

@robinmoussu
Copy link
Author

Sorry, what I meant was that I don't know if the markdown parser that we currently use gets confused if the end-ticks for a code block are not exactly three backticks and nothing else on that line. If it got confused by that, that'd pose a problem.

Of course it doesn't work!

warning: could not parse code block as Rust code
   --> src/foo.rs:100:5
    |
100 |   /// ``` rust
    |  _____^
101 | | /// let x = 1;
102 | | /// ``` pause
103 | | ///
104 | | /// ``` resume
105 | | /// let y = 2;
106 | | /// ```
    | |_______^
    |
    = note: error from rustc: unknown start of token: `
    = note: error from rustc: unknown start of token: `
    = note: error from rustc: unknown start of token: `
    = note: error from rustc: unknown start of token: `
    = note: error from rustc: unknown start of token: `
    = note: error from rustc: unknown start of token: `

Is this a real issue? I knew I was going to have to modify it to support the pause and resume anyway.

What happens if the second "part" has no_run (but not the first)?

Maybe I'm wrong, but I thought that you can have only one tag (rust, text, no_run, …) at a time. The first one can have anything, but the second one will be required to have resume`. So it's not an accumulation, but a propagation (and you can't have multiple tag by construction since any latter "parts" have the tag "resume").

If my hypothesis is wrong (you can have multiple tags), then they shouldn't automatically propagate (unlike what I said), and the exact interaction need indeed to be examined.


I also forget to explicitly say that a pause must be followed by a resume before the end of the documentation block, and that a resume must follow a pause in the same documentation block. Between those pause and resume you can have anything, including other code block, but no pause.

This means that you can have pauseresumepauseresume, but not a nested pausepauseresumeresume.

@RustyYato
Copy link

@burdges

You put them in #[cfg(test)] mod tests { } modules inside the regular source?

Yes, although as it has been noted, this may lead to some duplication.

@jonhoo
Copy link
Contributor

jonhoo commented Apr 3, 2020

Of course it doesn't work!

You can't have spaces between the backticks and the keyword, so that explains why that particular example doesn't work. I think in general in markdown though, the ending code fence isn't expected to have keywords. I don't know how hard it would be to fix that.

I knew I was going to have to modify it to support the pause and resume anyway.

You should (in theory) not have to change the markdown parser to add new code fence keywords/tags. It just passes the string it finds after the opening fence to you, and then leaves it up to you to derive semantics from it. Making it look for keywords in closing fences as well might require changing the actual Markdown standard and parser syntax though, which is likely a bigger (though not insurmountable) task.

Maybe I'm wrong, but I thought that you can have only one tag

Nope, you can have multiple separated by comma, such as

```rust,ignore,should_panic

@robinmoussu
Copy link
Author

Nope, you can have multiple separated by comma, such as

Interesting. Then I think that I need to take a closer look to the possible interaction between fences. First of all fences shouldn't propagate automatically unlike what I said initially. This also means that if adding fences at the end is complicated, we can add the pause at the beggining. Then probably rename pause to something like merge_next and resume to merge_previous (with names that better explain that anything can be in-between than previous/next).

@burdges
Copy link

burdges commented Apr 3, 2020

It'd breaks playground links if you did macros in #[cfg(test)] mod tests { } presumably, which sounds more annoying than repetition.

@robinmoussu
Copy link
Author

Speaking of playground links, is it an option, something currently being implemented, a RFC, a wish? I can't find it anywhere.

@robinmoussu robinmoussu force-pushed the master branch 3 times, most recently from 1e3f9dc to b8206ca Compare April 5, 2020 09:15
- Rename the feature name
- Use `merge_next`/`merge_previous` instead of documentation in documentation
- Do not propagate tags anymore
- Clarify the interaction with the various tags (like `compile_fail`, or `text`)
- Add the use of macro as possible alternative
@robinmoussu
Copy link
Author

(Sorry for the spam, I forgot that the PR were updated automatically when I pushed a draft to my personal clone).

So I updated my proposition to include all the changes that were suggested:

  • Rename the feature name
  • Use merge_next/merge_previous instead of documentation in documentation
  • Do not propagate tags anymore
  • Clarify the interaction with the various tags (like compile_fail, or text)
  • Add the use of macro as possible alternative

In the end, I think that all commits should be squashed, but for the moment it may be easier to keep them separated for an easier review.

@robinmoussu robinmoussu changed the title Initial version of documentation-in-code-block-in-documentation Code block continuation in documentation Apr 5, 2020
@Nemo157
Copy link
Member

Nemo157 commented Apr 9, 2020

Making it look for keywords in closing fences as well might require changing the actual Markdown standard and parser syntax though, which is likely a bigger (though not insurmountable) task.

Yes, the standard doesn't currently allow keywords following the closing fence:

The closing code fence may be indented up to three spaces, and may be followed only by spaces, which are ignored.

CommonMark Spec 0.29 § 4.5 ¶ 4

I personally don't see much utility in tagging the earlier block, it seems like a marker at the beginning to merge onto the previous block would be enough.

@robinmoussu
Copy link
Author

If you don't tag the previous block, you can't insert other (unrelated) code block in the middle.

@Screwtapello
Copy link

One block could start with:

```rust,ignore,should_panic,block=01

...then you could have some other code-blocks, and eventually a block that starts with:

```continue=01

All the blocks with continue=X need to be concatenated to the end of the block with block=X to create a full, compilable example. If there's more than one block with block=X in the same docstring, that's an error.

@burdges
Copy link

burdges commented Apr 9, 2020

We've no need for block= here since rust's own structuring handles that adequately once resume exists. In fact, almost anytime you want interleaved examples then you've interacting state machines for which one long block makes more sense.

We've discussed restarting multiple times for identical initialization code, so our blocks expand into a tree. I'm confident developers would exploit trees like this, unlike block= alone. Also trees give the nicest play ground links. In the code, one long block with macro_rules! might prove more readable than trees because everybody knows basic macros, ala #2894 (comment)

@mightyiam
Copy link

I and @arifd have analyzed the discussion and would like to summarize and provide our input.

Here's the current RFC's goal description:

The goal is to improve the experience of displaying complex examples in the documentation.

We would phrase it this way, instead:


Long code examples often call for explanatory text interspersed among them.
Currently, there are three means of achieving this:

  1. Break apart long code examples into multiple code blocks and write explanatory text in between them.
    The downside of this is that the example will no longer compile and run as a doc-test.
    Another downside is a lack of inherent UI that indicates that the code blocks continue each other.
  2. Write explanatory text as rust comments inside the long example.
    The downside of this is that it's less maintainable and less readable.
    Also, it does not allow for markdown features in that text.
  3. The current documented workaround which seems tedious. Macros may make this more reasonable, but still not nice.

These three means seem like workarounds and lacking in elegance.
This RFC suggests a feature that achieves this goal (explanatory text interspersed among code examples) while:

  1. Not sacrificing their running as doc-tests
  2. Providing UI that indicates that code blocks are members of a whole continuous example

Notice that this re-definition is not about reusing code between examples and therefore does not attempt to provide solutions for that purpose. This is mentioned regarding suggestions in that direction, such as #2894 (comment).

Two major design ideas have been identified:

  • Design idea "trees", where multiple blocks may continue from a same prior block.
  • Design idea "linear", where each block may be a member of only one multi-block code example.
    • Additional possible "interleaved" feature, where the blocks of a multi-part example may interleave with blocks of single or other multi-part examples.

Documentation is pages that are consumed usually top to bottom. Adding code blocks that continue one another in some tree structure may seem like a good idea from the perspective of the developer/author but would probably not actually be nice for the reader.

The "linear" design seems like the best choice here, because it's simple and provides a feature that is easy to understand and use both for the author and for the reader. Note that we would recommend some additional UI component that indicates which blocks belong to a continuation. Something like "block x of n" at the corner of each such block.

Regarding the additional possible "interleaved" feature, we suggest not implementing interleaving of multiple multi-part examples at this stage. However, there seems to be no reason to disallow single-block rust examples between the parts of a multi-part example, even though that should probably be used sparingly and with care.

So while we don't see a real use case for interleaved multiple multi-part examples nor for tree structures, we may be wrong about any of those.
Therefore the suggested syntax is designed to be future-compatible with those features. Suggested syntax example:

``` split_start
let mut a = 1;
```

Some explanatory markdown.

``` split_continue
a = 2;
```

Any valid markdown, including code blocks (although not multi-part rust):

```
dbg!(3);
```

And the final, ending block:

``` split_end
dbg!(a);
```

We would also suggest the following title for this RFC:

Inherent support for multi-part code examples in rustdoc

@robinmoussu
Copy link
Author

Whaoo. Your understanding of my initial idea is much better than mine! I really love how you phrased it.

I don’t think I will have time in the short or mid-term future to work on this. @mightyiam if you think you have the time, and want to make this issue move forward, feel free to do so. If it’s easier to open a new PR, do it, ping me, and I will close this one.

@mightyiam
Copy link

A new PR is in the works. I'd keep this one open for now.

@mightyiam
Copy link

@robinmoussu and all interested: new PR: #3081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to rustdoc team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants