Skip to content

rustdoc unexpectedly formatted leading numeral as a list #92005

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

Closed
davepacheco opened this issue Dec 16, 2021 · 14 comments
Closed

rustdoc unexpectedly formatted leading numeral as a list #92005

davepacheco opened this issue Dec 16, 2021 · 14 comments
Labels
A-markdown-parsing Area: Markdown parsing for doc-comments C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@davepacheco
Copy link

I have a feeling I'm doing something wrong here so I apologize in advance if this is just noise. But the behavior I'm seeing seems surprising.

I tried this code:

//! 1. One
//!    1. Two
//!
//! This record means: user 123 has the "admin" role on the "project" with id
//! 234.  (Note.)

and I built this with cargo doc.

I expected to see this sentence together in one paragraph: "This record means: user 123 has the "admin" role on the "project" with id 234. (Note.)"

Instead, this happened:

rustdoc

Looking at the HTML, it's interpreted the leading "234." as a numbered list. Thinking this was some Markdown error, I tried it in the dingus but it does what I expected.

Meta

$ cargo version
cargo 1.56.0 (4ed5d137b 2021-10-04)
$ rustdoc --version --verbose
rustdoc 1.56.1 (59eed8a2a 2021-11-01)
binary: rustdoc
commit-hash: 59eed8a2aac0230a8b53e89d4e99d55912ba6b35
commit-date: 2021-11-01
host: x86_64-apple-darwin
release: 1.56.1
LLVM version: 13.0.0

On cargo 1.59.0-nightly (a359ce160 2021-12-14), I see almost the same behavior, but the "2" in 234 gets eaten:

rustdoc-nightly

@davepacheco davepacheco added the C-bug Category: This is a bug. label Dec 16, 2021
@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-markdown-parsing Area: Markdown parsing for doc-comments labels Dec 17, 2021
@camelid
Copy link
Member

camelid commented Dec 17, 2021

I imagine this is probably an upstream bug with pulldown-cmark. cc @marcusklaas

@camelid camelid added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Dec 17, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 17, 2021
@camelid
Copy link
Member

camelid commented Dec 17, 2021

I wonder if the leading 2 is actually in the HTML but is being cut off by some CSS rules. Can you post the relevant snippet of HTML for both rustdoc versions?

@davepacheco
Copy link
Author

With cargo 1.56.0 (4ed5d137b 2021-10-04):

<div class="docblock"><ol>
<li>One
<ol>
<li>Two</li>
</ol>
</li>
</ol>
<p>This record means: user 123 has the “admin” role on the “project” with id</p>
<ol start="234">
<li>(Note.)</li>
</ol>
</div>

With cargo 1.59.0-nightly (a359ce160 2021-12-14):

<div class="docblock"><ol>
<li>One
<ol>
<li>Two</li>
</ol>
</li>
</ol>
<p>This record means: user 123 has the “admin” role on the “project” with id</p>
<ol start="234">
<li>(Note.)</li>
</ol>
</div>

In case it's useful, this is very easily reproduced -- e.g., with https://github.com/davepacheco/rustdoc-test.

@camelid
Copy link
Member

camelid commented Dec 17, 2021

Thanks! So, indeed, it seems the 2 is being hidden by the CSS; it is actually emitted in the HTML.

@camelid
Copy link
Member

camelid commented Dec 18, 2021

Assigning priority as discussed in the prioritization working group.

@rustbot label: +P-medium -I-prioritize

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Dec 18, 2021
@marcusklaas
Copy link
Contributor

Ooh, this is an interesting one. While this is certainly surprising, I'm not 100% confident this is a bug in pulldown. I played around in dingus, the CommonMark reference implementation, and found that it also produced a list for this snippet:

a
1.  d

However, if the final line is changed to 2. d instead, it does not produce a list.

I will have to check the spec, because I can't remember there being different rules for lists that do not start at one.

@marcusklaas
Copy link
Contributor

marcusklaas commented Dec 19, 2021

It turns out there is a special rule for starting lists with the number one. From the spec:

In order to solve of unwanted lists in paragraphs with hard-wrapped numerals, we allow only lists starting with 1 to interrupt paragraphs.

And then there follow two tests that test exactly this distinction. Pulldown passes those tests, so I'm confused why it also produces a list for

1. a
    1. a

a
2. a

I will create an issue on the pulldown repo and investigate further...

@marcusklaas
Copy link
Contributor

We have found the underlying issue and created a PR to address it here. Thanks to @camelid for flagging it with us!

@camelid
Copy link
Member

camelid commented Dec 19, 2021

Thanks for fixing it!

@camelid
Copy link
Member

camelid commented Dec 19, 2021

@marcusklaas what is the timeline for the next pulldown release?

@marcusklaas
Copy link
Contributor

Very soon!

@marcusklaas
Copy link
Contributor

Pulldown version 0.9.0 has just been released!

@camelid
Copy link
Member

camelid commented Dec 22, 2021

Thanks!

@camelid
Copy link
Member

camelid commented Dec 30, 2021

This will be fixed once #92252 has landed, which will happen shortly. Closing this issue.

EDIT: The PR has landed now.

@camelid camelid closed this as completed Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-markdown-parsing Area: Markdown parsing for doc-comments C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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

4 participants