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

rustdoc: Update pulldown-cmark to 0.8 #76687

Closed
camelid opened this issue Sep 13, 2020 · 17 comments · Fixed by #76689
Closed

rustdoc: Update pulldown-cmark to 0.8 #76687

camelid opened this issue Sep 13, 2020 · 17 comments · Fixed by #76689
Assignees
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Sep 13, 2020

Thanks to @marcusklaas's great work, pulldown-cmark now supports smart punctuation! This means that rustdoc can now have smart apostrophes, dashes, etc.


@rustbot modify labels: T-rustdoc C-enhancement

@rustbot rustbot added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 13, 2020
@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

It looks like there are a few breaking changes in the API:

$ ./x.py build --stage 1
error[E0308]: mismatched types
   --> src/librustdoc/html/markdown.rs:942:72
    |
934 |           let replacer = |_: &str, s: &str| {
    |  ________________________-
935 | |             if let Some(link) = links.iter().find(|link| &*link.original_text == s) {
936 | |                 Some((link.href.clone(), link.new_text.clone()))
937 | |             } else {
938 | |                 None
939 | |             }
940 | |         };
    | |_________- the found closure
941 | 
942 |           let p = Parser::new_with_broken_link_callback(md, opts(), Some(&replacer));
    |                                                                          ^^^^^^^^^ types differ in mutability
    |
    = note: expected mutable reference `&mut dyn for<'r> std::ops::FnMut(pulldown_cmark::BrokenLink<'r>) -> std::option::Option<(pulldown_cmark::CowStr<'_>, pulldown_cmark::CowStr<'_>)>`
                       found reference `&[closure@src/librustdoc/html/markdown.rs:934:24: 940:10 links:_]`

error[E0308]: mismatched types
    --> src/librustdoc/html/markdown.rs:1023:18
     |
1012 |           let replacer = |_: &str, s: &str| {
     |  ________________________-
1013 | |             if let Some(link) = links.iter().find(|link| &*link.original_text == s) {
1014 | |                 Some((link.href.clone(), link.new_text.clone()))
1015 | |             } else {
1016 | |                 None
1017 | |             }
1018 | |         };
     | |_________- the found closure
...
1023 |               Some(&replacer),
     |                    ^^^^^^^^^ types differ in mutability
     |
     = note: expected mutable reference `&mut dyn for<'r> std::ops::FnMut(pulldown_cmark::BrokenLink<'r>) -> std::option::Option<(pulldown_cmark::CowStr<'_>, pulldown_cmark::CowStr<'_>)>`
                        found reference `&[closure@src/librustdoc/html/markdown.rs:1012:24: 1018:10 links:_]`

error[E0308]: mismatched types
    --> src/librustdoc/html/markdown.rs:1091:72
     |
1087 |           let push = |_: &str, s: &str| {
     |  ____________________-
1088 | |             shortcut_links.borrow_mut().push((s.to_owned(), locate(s)));
1089 | |             None
1090 | |         };
     | |_________- the found closure
1091 |           let p = Parser::new_with_broken_link_callback(md, opts(), Some(&push));
     |                                                                          ^^^^^ types differ in mutability
     |
     = note: expected mutable reference `&mut dyn for<'r> std::ops::FnMut(pulldown_cmark::BrokenLink<'r>) -> std::option::Option<(pulldown_cmark::CowStr<'_>, pulldown_cmark::CowStr<'_>)>`
                        found reference `&[closure@src/librustdoc/html/markdown.rs:1087:20: 1090:10 shortcut_links:_, locate:_]`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
error: could not compile `rustdoc`.

@marcusklaas
Copy link
Contributor

Yup, the broken link callback type changed. Instead of two strings, you are now passed a BrokenLink, which contains the reference that was previously passed, amongst other things. It also expects you to return CowStrs, which can be created by calling Into::into on chars, &strs or Strings. Finally, we now accept FnMuts, which means that we need a mutable reference to the callback instead of a regular reference.

Fundamentally, it works the same way as before. It should be relatively straightforward to make the changes. Hope the above helps!

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

Okay, thanks!

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

@rustbot claim

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

Hmm, I fixed the breaking changes, but it doesn't seem to be working:

image

That was generated by build/x86_64-apple-darwin/stage1/bin/rustdoc after I built it from this Markdown:

# Test of "smart punctuation"

Hello there -- this is "smart punctuation"!

@marcusklaas
Copy link
Contributor

I can't see the PR, but did you remember to enable the smart punctuation option? It's not enabled by default.

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

I haven't pushed it up yet, so that's why :)

Ah, I didn't realize it was a parser option, I thought it was enabled by default or a crate-level feature. Thanks!

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

That worked, but it looks like smart punctuation doesn't work in headers, is that intentional?

image

@marcusklaas
Copy link
Contributor

Erm, not that I know. I will investigate for a bit.

@marcusklaas
Copy link
Contributor

I cannot reproduce this on pulldown itself -- seems to be working in titles there. Maybe rustdoc is doing something to titles? (I know nothing of rustdoc)

@camelid
Copy link
Member Author

camelid commented Sep 13, 2020

Okay, thanks for looking into it! Maybe rustdoc has some kind of custom header renderer.

@Dylan-DPC-zz
Copy link

cc @GuillaumeGomez

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

Oops, I made the same PR without noticing you were working on this: #76689

@camelid
Copy link
Member Author

camelid commented Sep 14, 2020

Your PR does not seem to enable the smart punctuation option though. How about we keep your PR for upgrading, and then I'll work on enabling smart punctuation (there seem to be some issues with how rustdoc handles headers)?

@rustbot

This comment has been minimized.

@camelid

This comment has been minimized.

@jyn514 jyn514 assigned jyn514 and unassigned camelid Sep 14, 2020
@camelid
Copy link
Member Author

camelid commented Sep 14, 2020

Are you the new rustbot? ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants