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 (Rustdoc): hoedown -> pulldown migration #44229

Closed
nrc opened this issue Aug 31, 2017 · 40 comments
Closed

Tracking issue (Rustdoc): hoedown -> pulldown migration #44229

nrc opened this issue Aug 31, 2017 · 40 comments

Comments

@nrc
Copy link
Member

@nrc nrc commented Aug 31, 2017

If you've arrived via a warning and what to know what is going on, see this comment.

  • implement Pulldown rendering behind a flag (#40338, #41431)
  • implement warnings for changes (#41991)
  • begin warning cycle (#45324)
  • move to Pulldown by default (#47398)
  • remove Hoedown, the difference warnings, the html-diff dependency, etc (#48274)

Blocking warning cycle

@nrc
Copy link
Member Author

@nrc nrc commented Aug 31, 2017

cc @rust-lang/dev-tools @rust-lang/docs

@nrc
Copy link
Member Author

@nrc nrc commented Aug 31, 2017

What is happening?

We're changing the way that Rustdoc renders markdown in your docs. We're moving to use a new parser and renderer (Pulldown) which is standards compliant (CommonMark), more robust, less buggy, and written in Rust. You can use Pulldown on nightly rustdoc by using the --enable-commonmark command line argument.

However, there are some changes to how your docs might be rendered. The known issues are:

  • Pulldown does not support superscripts such as 2^5, you must use 2<sup>5</sup> instead.
  • Pulldown identifies more examples as code blocks
  • Pulldown identifies numbered lists in more places than Hoedown
  • Pulldown requires a newline between a paragraph and any link reference definitions
  • 'automatic links' in Pulldown need <> around them
  • Hoedown required a newline between paragraphs and lists, Pulldown does not.
  • A space is required after ** to work as emphasis in Pulldown
  • Many insignificant changes to whitespace in generated HTML, etc.

We are currently providing warnings (when running Rustdoc) if the documentation for your crate will change with the new renderer. You should check these warnings to ensure that your documentation still renders correctly.

bors added a commit that referenced this issue Sep 1, 2017
Improve the Pulldown/hoedown warnings

cc #44229

r? @QuietMisdreavus
bors added a commit that referenced this issue Sep 1, 2017
Improve the Pulldown/hoedown warnings

cc #44229

r? @QuietMisdreavus
@nrc
Copy link
Member Author

@nrc nrc commented Sep 5, 2017

Update: #44238 and #44329 addressed some issues with the warnings and it is now off by default (it was on by default for one nightly release). The next step is to assess the false positive situation. @QuietMisdreavus is going to list some good test crates and obtain the warning output for them. @GuillaumeGomez is going to try some experiments to reduce the false positive rate.

@nrc
Copy link
Member Author

@nrc nrc commented Sep 14, 2017

ping @QuietMisdreavus did you get a chance to look at good test crates?

@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 14, 2017

After getting the first false positive i didn't dig much farther. However, a quick check of a handful of popular crates shows some more leads:

time looks like it has some true positives, in the form of ^ not being <sup>:

 Documenting time v0.1.38
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `Simple time handling....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/time-0.1.38/src/lib.rs:11:0
    /html[0]/body[1]/p[6] Text differs:
        expected: `... [strftime()]...`
        found:    `...`
    /html[0]/body[1]/p[6] Unexpected element `a`: found: `<a href="http://man7.org/linux/man-pages/man3/strf ... html">strftime()</a>`
    /html[0]/body[1]/p[6] Unexpected element: ` function from the C...`
WARNING: rendering difference in `Identifies the time zone that was used to compute this broken-down time...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/time-0.1.38/src/lib.rs:336:4
    /html[0]/body[1]/p[0] Text differs:
        expected: `...`
        found:    `...*60*60 = -25200.`
    /html[0]/body[1]/p[0] One element is missing: expected: `em`
    /html[0]/body[1]/p[0] One element is missing: expected: `0`
WARNING: rendering difference in `Returns the total number of whole microseconds in the duration,...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/time-0.1.38/src/duration.rs:197:4
    /html[0]/body[1]/p[0] Text differs:
        expected: `on overflow (exceeding 2^63 microseconds in either direction).`
        found:    `on overflow (exceeding 2`
    /html[0]/body[1]/p[0] Unexpected element `sup`: found: `<sup>63</sup>`
    /html[0]/body[1]/p[0] Unexpected element: ` microseconds in either direction).`
WARNING: rendering difference in `Returns the total number of whole nanoseconds in the duration,...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/time-0.1.38/src/duration.rs:205:4
    /html[0]/body[1]/p[0] Text differs:
        expected: `on overflow (exceeding 2^63 nanoseconds in either direction).`
        found:    `on overflow (exceeding 2`
    /html[0]/body[1]/p[0] Unexpected element `sup`: found: `<sup>63</sup>`
    /html[0]/body[1]/p[0] Unexpected element: ` nanoseconds in either direction).`

mio looks like it gets "out of sync" when looking at link tags? This probably requires a closer look.

 Documenting mio v0.6.10
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `An readiness event returned by [`Poll::poll`]....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-0.6.10/src/event_imp.rs:941:0
    /html[0]/body[1]/p[1] Text differs:
        expected: `is a`
        found:    `is a [readiness state] paired with a`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Ready.html"}`, found: `{"href": "struct.Token.html"}`
    /html[0]/body[1]/p[1] Text differs:
        expected: `paired with a`
        found:    `. It is returned by`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Token.html"}`, found: `{"href": "struct.Poll.html#method.poll"}`
    /html[0]/body[1]/p[1] Text differs:
        expected: `. It is returned by`
        found:    `.`
    /html[0]/body[1]/p[1] One element is missing: expected: `a`
    /html[0]/body[1]/p[1] One element is missing: expected: ``
WARNING: rendering difference in `A collection of readiness events....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-0.6.10/src/poll.rs:1180:0
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[2] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
WARNING: rendering difference in `A collection of readiness events....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-    0.6.10/src/poll.rs:1180:0
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
    /html[0]/body[1]/p[2] Attributes differ in `a`: expected: `{"href": "struct.Poll.html#method.poll"}`, found: `{"href": "struct.Poll.html"}`
WARNING: rendering difference in `Set the registration's readiness...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/mio-0.6.10/src/poll.rs:1614:4
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html"}`, found: `{"href": "struct.Poll.html#method.poll"}`
    /html[0]/body[1]/p[1] Attributes differ in `a`: expected: `{"href": "struct.Poll.html"}`, found: `{"href": "struct.Poll.html#method.poll"}`

Something similar happens in hyper as well:

 Documenting hyper v0.11.2 (file:///home/misdreavus/clones/hyper)
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `Percent encode a sequence of bytes with a character set defined in...`
   --> src/header/parsing.rs:153:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `......`
        found:    `...`
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc5987#secti ... 5987#section-3.2</a>`
WARNING: rendering difference in `The `Pragma` header defined by HTTP/1.0....`
   --> src/header/common/pragma.rs:34:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `Spec: https://tools.ietf.org/html/rfc7234#section-5.4`
        found:    `Spec:`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc7234#secti ... 7234#section-5.4</a>`
WARNING: rendering difference in `A Media Descriptors Enum based on:...`
   --> src/header/common/link.rs:112:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `......`
        found:    `...`
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://www.w3.org/TR/html401/types.html# ... ypes.html#h-6.13</a>`
WARNING: rendering difference in `A Mime charset....`
   --> src/header/shared/charset.rs:14:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `... http://www.iana.org/assignments/character-sets/character-sets.xhtml`
        found:    `...`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="http://www.iana.org/assignments/character ... acter-sets.xhtml</a>`
WARNING: rendering difference in `The `Origin` header....`
   --> src/header/common/origin.rs:35:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `... https://fetch.spec.whatwg.org/#origin-header, the value of this header is composed of...`
        found:    `...`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="https://fetch.spec.whatwg.org/#origin-hea ... g/#origin-header</a>`
    /html[0]/body[1]/p[2] Unexpected element: `, the value of this header is composed of...`
WARNING: rendering difference in `The scheme, such as http or https...`
   --> src/header/common/origin.rs:76:4
    /html[0]/body[1]/p[0] Unexpected element `code`: found: `<code>use hyper::header::Origin; let origin = Orig ... me("https"));</code>`
    /html[0]/body[1] One element is missing: expected: `pre`
WARNING: rendering difference in `The host, such as Host{hostname: "hyper.rs".to_owned(), port: None}...`
   --> src/header/common/origin.rs:89:4
    /html[0]/body[1]/p[0] Unexpected element `code`: found: `<code>use hyper::header::{Origin,Host}; let origin ... Some(443))));</code>`
    /html[0]/body[1] One element is missing: expected: `pre`

I'll add more samples over time.

@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 14, 2017

The results for webpki allude to the source of the link tag issue: It looks like one parser automatically linkifies things in inline code spans?

 Documenting webpki v0.17.0
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `webpki: Web PKI X.509 Certificate Validation....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/webpki-0.17.0/src/webpki.rs:15:0
    /html[0]/body[1]/p[1]/code[0] Text differs:
        expected: `git clone https://github.com/briansmith/webpki`
        found:    `git clone`
    /html[0]/body[1]/p[1]/code[0] Unexpected element `a`: found: `<a href="https://github.com/briansmith/webpki">htt ... riansmith/webpki</a>`
WARNING: rendering difference in `RSA PSS signatures using SHA-512 for keys of 2048-8192 bits and of...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/webpki-0.17.0/src/signed_data.rs:263:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `... https://tools.ietf.org/html/rfc4055#section-1.2`
        found:    `...`
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc4055#secti ... 4055#section-1.2</a>`
WARNING: rendering difference in `RSA PSS signatures using SHA-384 for keys of 2048-8192 bits and of...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/webpki-0.17.0/src/signed_data.rs:254:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `... https://tools.ietf.org/html/rfc4055#section-1.2`
        found:    `...`
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc4055#secti ... 4055#section-1.2</a>`
WARNING: rendering difference in `RSA PSS signatures using SHA-256 for keys of 2048-8192 bits and of...`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/webpki-0.17.0/src/signed_data.rs:245:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `... https://tools.ietf.org/html/rfc4055#section-1.2`
        found:    `...`
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc4055#secti ... 4055#section-1.2</a>`
@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 14, 2017

On a farther read, the webpki may be a true rendering difference: The line being rendered there doesn't use backticks or indentation: it's a manual <code> span written into the docs. This may have been a deliberate decision to have the link in the code span, but it seems that pulldown suppresses the link generation that hoedown did, even when it's a manual <code> tag.

EDIT: gonna add farther notes to this comment rather than spamming the thread.

time's rendering differences seem to all be true differences: The aforementioned <sup> change, a link anchor being spread across multiple lines (proper link in hoedown, broken tag in pulldown), and asterisks inline to a word being treated differently (hoedown left them as asterisks, pulldown created an <em> tag for them).

EDIT 2: mio is also all legit. The last three are also about the case-insensitivity of reference links. The first is a new difference: link reference tags are whitespace-stripped in Pulldown, but not Hoedown. The published 0.6.10 has no link for "readiness state" because the anchor added a space before the closing bracket. Pulldown stripped that space and completed the link, hence getting "out of sync" because there's an anchor tag where there wasn't one before.

EDIT 3: hyper is also all legit. Several URLs that aren't auto-linked in Pulldown, and a couple places where there wasn't a newline between a regular paragraph and a ``` code block.

EDIT 4: mime introduces yet another new difference:

 Documenting mime v0.3.3 (file:///home/misdreavus/clones/mime)
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `*`
   --> src/lib.rs:503:8
    /html[0]/body[1] Tags differ: expected: `ul`, found: `p`

Here, the entire docstring is a single asterisk. Hoedown leaves it alone, puts it into the output as-is. Pulldown creates a new unordered list with an single, empty item.

@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 14, 2017

rocket (the crate i tried to start with before i started rummaging through its dependencies) has a fine stack of warnings:

 Documenting rocket v0.3.2 (file:///home/misdreavus/clones/rocket)
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `Application configuration and configuration parameter retrieval....`
   --> src/config/mod.rs:1:0
    /html[0]/body[1]/ul[10]/li[5] Tags differ: expected: `strong`, found: `p`
    /html[0]/body[1]/ul[10]/li[5] Types differ: expected: ``, found: `ol`
    /html[0]/body[1]/ul[10]/li[5] Tags differ: expected: `em`, found: `ul`
    /html[0]/body[1]/ul[10]/li[5] One element is missing: expected: ``
    /html[0]/body[1]/ul[10]/li[5] One element is missing: expected: `ol`
    /html[0]/body[1]/ul[10]/li[5] One element is missing: expected: `ul`
    /html[0]/body[1]/ul[10]/li[6] Tags differ: expected: `strong`, found: `p`
    /html[0]/body[1]/ul[10]/li[6] Types differ: expected: ``, found: `ul`
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: `em`
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: ``
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: `em`
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: ``
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: `em`
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: ``
    /html[0]/body[1]/ul[10]/li[6] One element is missing: expected: `ul`
WARNING: rendering difference in `The `Pragma` header defined by HTTP/1.0....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.13/src/header/common/pragma.rs:34:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `Spec: https://tools.ietf.org/html/rfc7234#section-5.4`
        found:    `Spec:`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="https://tools.ietf.org/html/rfc7234#secti ... 7234#section-5.4</a>`
WARNING: rendering difference in `A Mime charset....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.13/src/header/shared/charset.rs:14:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `... http://www.iana.org/assignments/character-sets/character-sets.xhtml`
        found:    `...`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="http://www.iana.org/assignments/character ... acter-sets.xhtml</a>`
WARNING: rendering difference in `The `Origin` header....`
   --> /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/hyper-0.10.13/src/header/common/origin.rs:34:0
    /html[0]/body[1]/p[2] Text differs:
        expected: `... https://fetch.spec.whatwg.org/#origin-header, the value of this header is composed of...`
        found:    `...`
    /html[0]/body[1]/p[2] Unexpected element `a`: found: `<a href="https://fetch.spec.whatwg.org/#origin-hea ... g/#origin-header</a>`
    /html[0]/body[1]/p[2] Unexpected element: `, the value of this header is composed of...`
WARNING: rendering difference in `Returns the Media-Type associated with the extension `ext`. Not all...`
   --> src/http/media_type.rs:168:8
    /html[0]/body[1] Tags differ: expected: `h1`, found: `p`
WARNING: rendering difference in `Media type for <b>...`
   --> src/http/media_type.rs:115:12
    /html[0]/body[1]/p[0]/i[1] Unexpected element `*
/
*
`: found: `...`
    /html[0]/body[1] One element is missing: expected: `i`
    /html[0]/body[1] One element is missing: expected: `p`
WARNING: rendering difference in `Checks if the value is an ASCII punctuation charac ... s_ascii_punctuation)`
   --> /home/misdreavus/git/rust/src/libstd/ascii.rs:448:4
    /html[0]/body[1]/p[0]/code[2] Text differs:
        expected: `[ \\ ] ^ _ \`` U+007B ... U+007E`
        found:    `[ \\ ] ^ _ \`
    /html[0]/body[1]/p[0] Types differ: expected: ``, found: `code`
    /html[0]/body[1]/p[0] Types differ: expected: `a`, found: ``
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="https://doc.rust-lang.org/nightly/std/asc ... ation">Read more</a>`
WARNING: rendering difference in `An `Accept` header with the single media type for <b>...`
   --> src/http/accept.rs:157:12
    /html[0]/body[1]/p[0]/i[2] Unexpected element `*
/
*
`: found: `...`
    /html[0]/body[1] One element is missing: expected: `i`
    /html[0]/body[1] One element is missing: expected: `p`
WARNING: rendering difference in `Content-Type for <b>...`
   --> src/http/content_type.rs:54:12
    /html[0]/body[1]/p[0]/i[1] Unexpected element `*
/
*
`: found: `...`
    /html[0]/body[1] One element is missing: expected: `i`
    /html[0]/body[1] One element is missing: expected: `p`

In sum, rocket continues to deliver as the quintessential rustdoc stress test. There are a few new rendering differences in here, a couple of which are technically bugs in pulldown.

  • When there is a blank line in the middle of a list, pulldown will only wrap the items after that blank line with <p> tags - this is a bug in pulldown, as the commonmark spec asks to put every item in a "loose" list in a <p> tag
    • Hoedown has similar behavior, but also puts a <p> tag around the item immediately before the blank line
  • Hoedown does not treat a header marker as a heading unless it's at the beginning of the line (without leading spaces); pulldown does not care about the leading spaces (until it becomes a code block, of course)
    • Note that this fixes #42760 without having to change rustdoc itself to handle it
  • When there is a raw HTML tag wrapping a section of multiple lines, pulldown still treats lines with only asterisks on them as unordered lists - this is technically a bug, as the commonmark.js dingus performs the same as hoedown, which renders each asterisk plainly
@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Sep 14, 2017

I did all of this testing using the approved-but-not-yet-merged #44368. Judging from how all of these turned out, i'm willing to say that as soon as that lands, all the rendering warnings left will be true rendering differences. It doesn't mean that Pulldown is perfect, just that we're accurately talking about what it's doing.

@nrc
Copy link
Member Author

@nrc nrc commented Sep 18, 2017

Next step here is that @GuillaumeGomez will draft a blog post about how to address the changes required for crate authors. We'll advertise that as widely as possible and then (depending on fallout) change the warnings to on by default a few weeks later.

mbrubeck added a commit to mbrubeck/rust-siphash that referenced this issue Oct 17, 2017
This fixes formatting problems when the Markdown processor is switched
to pulldown-cmark (rust-lang/rust#44229).
mbrubeck added a commit to mbrubeck/app_units that referenced this issue Oct 17, 2017
This fixes formatting problems when the Markdown processor is switched
to pulldown-cmark (rust-lang/rust#44229).
mbrubeck added a commit to mbrubeck/image that referenced this issue Oct 17, 2017
This makes documentation work correctly with the new pulldown-cmark
Markdown parser (rust-lang/rust#44229).
mbrubeck added a commit to mbrubeck/num that referenced this issue Oct 17, 2017
This makes formatting correct with the new pulldown-cmark Markdown
parser (rust-lang/rust#44229).
@Manishearth Manishearth mentioned this issue Dec 28, 2017
6 of 12 tasks complete
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Dec 28, 2017

Are we still waiting on something to flip the switch? #47046 would be slightly easier if we didn't have to support both. Slightly, not very, so don't rush on its account.

@steveklabnik
Copy link
Member

@steveklabnik steveklabnik commented Dec 28, 2017

It hasn't hit stable yet; we wanted to do that before we started making more moves.

MaloJaffre added a commit to MaloJaffre/parking_lot that referenced this issue Dec 29, 2017
Lists now require an empty line before them, and a link can no longer be
split on multiple lines.
See rust-lang/rust#44229.
@nrc nrc mentioned this issue Jan 8, 2018
26 of 27 tasks complete
@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Jan 12, 2018

With the release of 1.23.0 last week, the warnings-by-default have hit stable! Since we haven't heard much by way of people reacting to it, we're moving on with the next step: rendering with Pulldown by default, with an option to go back to Hoedown. This is being done in #47398.

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jan 13, 2018

I checked the checkbox (to avoid someone doing a duplicate).

@sunjay
Copy link
Member

@sunjay sunjay commented Jan 14, 2018

Sorry if this isn't the right place to ask. I'm trying to figure out how to migrate turtle's documentation to the new renderer. There are lots of warnings but I'm not really clear on how to fix anything. Is there a way for me to see the actual output differences? There are some weird things going on where rustdoc seems to be rendering ol tags in places where there really shouldn't be any. The method at turtle.rs:344 doesn't actually have any lists as far as I know, but rustdoc seems to think there should be? Also, there seem to be some differences with p and table tags that I am not clear about how to fix.

$ cargo doc --no-deps
WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See https://github.com/rust-lang/rust/issues/44229 for details.
WARNING: rendering difference in `Represent a keyboard key.`
   --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/pistoncore-input-0.20.0/src/keyboard.rs:109:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `... http://wiki.libsdl.org/SDLKeycodeLookup`
        found:    `...`
WARNING: rendering difference in `Represent a keyboard key.`
   --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/pistoncore-input-0.20.0/src/keyboard.rs:109:0
    /html[0]/body[1]/p[0] Unexpected element `a`: found: `<a href="http://wiki.libsdl.org/SDLKeycodeLookup"> ... SDLKeycodeLookup</a>`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `ol`, found: `p`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `p`, found: `pre`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `pre`, found: `p`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `p`, found: `table`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `table`, found: `p`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `p`, found: `h1`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `h1`, found: `p`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `p`, found: `h1`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] Tags differ: expected: `h1`, found: `p`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `Using this method is an excellent way to learn about conversion`
        found:    `We have implemented that trait for several types l ... bit integers so that`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1]/code[0] Text differs:
        expected: `From`
        found:    `Speed::Six`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `and`
        found:    `directly, you can use just`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1]/code[1] Text differs:
        expected: `Into`
        found:    `6`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `... This method takes a`
        found:    `...`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Tags differ: expected: `em`, found: `code`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `...its speed parameter. That type`
        found:    `...provided by the`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1]/code[2] Text differs:
        expected: `Into`
        found:    `Into<Speed>`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `...for the type`
        found:    `...to get the`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] Text differs:
        expected: `. That means that`
        found:    `value.`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] One element is missing: expected: `em`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] One element is missing: expected: ``
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] One element is missing: expected: `code`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[1] One element is missing: expected: ``
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] Text differs:
        expected: `We have implemented that trait for several types l ... bit integers so that`
        found:    `You can pass in strings, 32-bit integers, and even`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2]/code[0] Text differs:
        expected: `Speed::Six`
        found:    `Speed`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] Text differs:
        expected: `directly, you can use just`
        found:    `enum variants because they all`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2]/code[1] Text differs:
        expected: `6`
        found:    `Into<Speed>`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] Text differs:
        expected: `.`
        found:    `trait.`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: `code`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: ``
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: `code`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: ``
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: `code`
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1]/p[2] One element is missing: expected: ``
WARNING: rendering difference in `Set the turtle's movement speed to the given setti ... cts the animation of`
   --> src/turtle.rs:344:4
    /html[0]/body[1] One element is missing: expected: `p`

Also, smaller issue, the warnings for certain dependencies still seem to be outputted even with --no-deps. Not sure why.

@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Jan 14, 2018

There's no interactive difference view, but you can render the docs with the different renderers to see the differences. With a nightly, run once with cargo doc, copy those out, then run with cargo rustdoc -- -Z unstable-options --enable-commonmark to use the new renderer. Then, open both sets of docs and navigate to the pages pointed to by the file/line directives in the warning set.

@chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Jan 14, 2018

@sunjay
Copy link
Member

@sunjay sunjay commented Jan 14, 2018

There's no interactive difference view, but you can render the docs with the different renderers to see the differences.

That is exactly what I was looking for. Thanks! Though an integrated diff view would certainly be nice too, being able to manually do it myself is good enough for me.

It turns out that the ol issue I was describing was caused by me having a line start with a number and period. It was technically part of the previous sentence, but commonmark doesn't require an empty line before a list. Easy fix. Good thing these warnings were there!


In case it helps anyone else, here's what I did to find the differences between the renderers:

$ # Start clean by removing any previously generated docs
$ rm -rf target/doc*
$ # Generate docs with the old renderer
$ cargo +nightly doc --no-deps
$ mv target/doc target/doc-hoedown
$ # Generate docs with the new renderer
$ cargo +nightly rustdoc -- -Z unstable-options --enable-commonmark
$ # There are now two folders: `target/doc-hoedown` (old renderer) and `target/doc` (new renderer)
$ diff -r target/doc-hoedown/turtle target/doc/turtle

Note that turtle is the name of my crate, so anyone else using these instructions should replace turtle with whatever they are trying to diff.

@steveklabnik
Copy link
Member

@steveklabnik steveklabnik commented Jan 14, 2018

@chriskrycho too much, as we're planning on phasing the whole thing out very soon. Very few people at this point have commented so far, so it seems that the majority of issues have already been taken care of. Lots of work for little reward.

@chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Jan 14, 2018

@steveklabnik 👍 I did not want to go after it if that was the case!

sunfishcode pushed a commit to sunfishcode/wasmparser.rs that referenced this issue Jan 16, 2018
This fixes warnings like the following when doing "cargo doc":

WARNING: documentation for this crate may be rendered differently using the new Pulldown renderer.
    See rust-lang/rust#44229 for details.
WARNING: rendering difference in `External types as defined at https://webassembly.g ... .html#external-types`
   --> src/parser.rs:104:0
    /html[0]/body[1]/p[0] Text differs:
        expected: `... https://webassembly.github.io/spec/syntax/types.html#external-types`
        found:    `...`
@slowli slowli mentioned this issue Jan 29, 2018
2 of 4 tasks complete
niluxv added a commit to niluxv/rust-crypto that referenced this issue Feb 3, 2018
@Michael-F-Bryan
Copy link

@Michael-F-Bryan Michael-F-Bryan commented Feb 4, 2018

I just noticed that tables aren't being rendered with the new pulldown renderer. I believe it's a trivial fix though (see raphlinus/pulldown-cmark#104).

For Example:

    /// Deserialize a raw CAN packet, validating its fields.
    ///
    /// # Format
    ///
    /// When deserializing the raw packet's payload, we expect it to be laid out
    /// using the following
    ///    
    /// | Offset   | Name   | Description                      |
    /// |----------+--------+----------------------------------|
    /// | 0        | Bus ID | Which bus this packet came in on |
    /// | 1        | CAN ID | The associated CAN ID            |
    /// | 3        | Length | The length of the data section   |
    /// | 4..      | Data   | The packet's payload             |
    pub fn parse(data: &[u8]) -> Result<(CanPacket, usize), Error> {
        ...
    }

When you viewed using cargo +stable doc:

screenshot_2018-02-04_130912

And on nightly:

screenshot_2018-02-04_130732

@QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Feb 4, 2018

We do enable tables in the Pulldown parser. Your row separating the header from the data isn't what it's expecting, though. See https://github.com/webuni/commonmark-table-extension for the syntax - it wants you to use | to separate the columns, but you're using + there.

@Ogeon
Copy link

@Ogeon Ogeon commented Feb 17, 2018

It seems like comments on trait methods get unescaped when they are shown as implemented traits. Something like a\*b\*c\* shows up as a*b*c* in the trait documentation, but as abc* when looking at a type that implements the trait. Adding double escapes makes it look bad on the trait page instead, and replacing * with &#42; doesn't help, since it gets reinterpreted as * anyway.

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Feb 18, 2018

@Ogeon: Please open a new issue for this.

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Feb 18, 2018

This #48274 got merged, the C cursed has now definitely been removed. Let's all celebrate by closing this issue! \o/

@Ogeon
Copy link

@Ogeon Ogeon commented Feb 18, 2018

@GuillaumeGomez Alright, submitted #48332.

ogham added a commit to ogham/rust-users that referenced this issue Mar 10, 2018
- Fix mismatched code blocks
- Remove newline from link (revealed by rust-lang/rust#44229)
yvt added a commit to yvt/ngspades that referenced this issue Apr 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants