As you may be aware, rustdoc has had some recent – and long awaited – improvements lately. @imperio has pushed through a change to use the pulldown rendering engine in place of hoedown. pulldown offers a number of bug fixes and moves us over to being “standards” (CommonMark) compliant – and, for bonus points, it’s written in Rust.
However, there is a wrinkle. Pulldown differs from hoedown in some particulars:
- There are various rendering differences you have to be aware of.
- For example, hoedown supports
2^5for superscripts, whereas pulldown requires
- More details below
- For example, hoedown supports
- Pulldown identifies codeblocks that hoedown ignored.
- This (initially) caused a fair amount of breakage where various bits of code were now being tested that were not before
- We’ve since landed a “transition” PR that issues warnings about such tests instead
- After some suitable period, these tests will execute by default.
There was a long discussion in the #rust-docs IRC channel this morning about how to phase this change in. In particular, is it sufficient to issue warnings for new tests, but allow the changes in rendering to silently change?
General policy: the three phases
Generally speaking when we have to make a change that changes behavior, we try to phase it in. The current “best practices” is to go through three general phases.
- “Opt-in”: keep old behavior, issue a targeted warning for those cases where behavior will change.
- Example: issue a future compatibility lint that is “warn by default”
- “Opt-out”: switch to new behavior by default, but allow users to “opt out” and get back the new behavior.
- Example: change future compatibility lint to “deny by default” (user can still set it back to warn)
- “Error”: switch to new behavior.
- Example: change future compatibility lint to a hard error.
As of PR 41290, we are following this scheme for tests, at least – but not for rendering, where we basically skipped ahead to stage 3.
How we could phase in rendering
If we wanted to go through the phases for rendering, it would presumably mean that we render things with both hoedown and pulldown and issue warnings where they seem to be generating different output. I think that, in an ideal world, we would have started out by landing exactly this, rather than just changing over to pulldown by default. However, this is not where we’re at.
@GuillaumeGomez feels that writing such a PR is definitely feasible – but there was concern that this would result in a lot of warnings. Given that the beta branch is going to occur soon, there may not be enough time to calibrate the warnings so that people are only getting warnings about important rendering changes, and not minor stuff. Our experience so far has been that most rendering results are unchanged. Therefore, we don’t want to be issuing tons of warnings over nothing.
@Snorge suggested that they already did some amount of “rendering diffs” preparing for the change, so perhaps they can comment more on how difficult that is to tune properly.
Known rendering differences
So far three differences in rendering have been found:
- superscripts must be written out using HTML (
- footnote definitions need an extra line before the definition (
- code blocks can occur in lists (see e.g. this diff, which demonstrates a comment that had to be changed)
UPDATE: More differences have come up in the comments:
TL;DR the question at hand
Beta is branching next week or so. The currently nightly implements this behavior:
- use hoedown for tests, warn about diffs with pulldown.
- use pulldown for rendering, which may differ in some particulars.
Is this satisfactory, or should we change something? The following options have been proposed:
- Nothing. The status quo is ok – tests continue to work, and we will phase in pulldown over time. Rendering is updated, but the effects are minor, and people will correct them over time.
- Revert the change completely and land it with phasing. We could back out the change and use hoedown again. Then we could land pulldown again, but this time in a more phased fashion, so that we begin with warnings about rendering differences. By backing out the change, we gain time to tinker with the warnings and make sure they are not too overwhelming.
- Try to land rendering warnings now. However, there isn’t much time left before beta, so there is some concern that this will result in a lot of unnecessary warnings being issued and an overall degraded user experience.