-
Notifications
You must be signed in to change notification settings - Fork 13.4k
librustdoc: Use correct heading levels. #89506
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
Changes from 1 commit
a8a40ea
4a6aa6e
6518a0a
13558ee
f1425c7
08a4f24
742d8be
1f86a8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#### Note: this error code is no longer emitted by the compiler. | ||
### Note: this error code is no longer emitted by the compiler. | ||
|
||
You tried to provide a lifetime to a type which doesn't need it. | ||
See `E0109` for more details. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
#### Note: this error code is no longer emitted by the compiler. | ||
### Note: this error code is no longer emitted by the compiler. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
//! | ||
//! let s = "My *markdown* _text_"; | ||
//! let mut id_map = IdMap::new(); | ||
//! let md = Markdown(s, &[], &mut id_map, ErrorCodes::Yes, Edition::Edition2015, &None); | ||
//! let md = Markdown(s, &[], &mut id_map, ErrorCodes::Yes, Edition::Edition2015, &None, 0); | ||
//! let html = md.into_string(); | ||
//! // ... something using html | ||
//! ``` | ||
|
@@ -47,6 +47,8 @@ use pulldown_cmark::{ | |
#[cfg(test)] | ||
mod tests; | ||
|
||
const MAX_HEADER_LEVEL: u32 = 6; | ||
|
||
/// Options for rendering Markdown in the main body of documentation. | ||
pub(crate) fn main_body_opts() -> Options { | ||
Options::ENABLE_TABLES | ||
|
@@ -78,6 +80,7 @@ pub struct Markdown<'a>( | |
/// Default edition to use when parsing doctests (to add a `fn main`). | ||
pub Edition, | ||
pub &'a Option<Playground>, | ||
pub u32, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a doc comment explaining what this field is. Out of context, it's impossible to know what it is without going through the code.
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
/// A tuple struct like `Markdown` that renders the markdown with a table of contents. | ||
crate struct MarkdownWithToc<'a>( | ||
|
@@ -489,11 +492,12 @@ struct HeadingLinks<'a, 'b, 'ids, I> { | |
toc: Option<&'b mut TocBuilder>, | ||
buf: VecDeque<SpannedEvent<'a>>, | ||
id_map: &'ids mut IdMap, | ||
level: u32, | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
impl<'a, 'b, 'ids, I> HeadingLinks<'a, 'b, 'ids, I> { | ||
fn new(iter: I, toc: Option<&'b mut TocBuilder>, ids: &'ids mut IdMap) -> Self { | ||
HeadingLinks { inner: iter, toc, buf: VecDeque::new(), id_map: ids } | ||
fn new(iter: I, toc: Option<&'b mut TocBuilder>, ids: &'ids mut IdMap, level: u32) -> Self { | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
HeadingLinks { inner: iter, toc, buf: VecDeque::new(), id_map: ids, level } | ||
} | ||
} | ||
|
||
|
@@ -530,6 +534,7 @@ impl<'a, 'b, 'ids, I: Iterator<Item = SpannedEvent<'a>>> Iterator | |
self.buf.push_front((Event::Html(format!("{} ", sec).into()), 0..0)); | ||
} | ||
|
||
let level = std::cmp::min(level + self.level + 1, MAX_HEADER_LEVEL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I would have expected that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Completely forgot about the standalone markdown files, this is an excellent point! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I wasn't feeling great about it either but since I'm not the only thinking this, we should definitely make the heading level match the level we use. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to be clear - you want markdown in the error index to behave differently ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why it makes sense to talk about them "behaving the same" as doc comments. They're totally different. For one thing, the biggest reason why this probably has to be fixed by changing rustdoc's behavior is that there's no way for the end-user to know what the right heading level is. The same exact doc comment might be rendered at multiple levels in multiple pages, like slice::first on the slice page (which is a fourth-level header, so the items in the doc comment should be h5), and slice::first on Vec (which is an h3, so its doc comment headers should start at h4). For the error index, you're probably in the right here. The error index pages each have h1's at the beginning, and when they're viewed as individual documents, this works (they're But for standalone markdown files, this seems like a breaking change made with insufficient motivation. Those aren't being combined to form huge mega-documents, and it's a stable feature, not just some internal thing used by the compiler, so changing its behavior should only be done with very compelling reasons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The short version: yes to changing doc comments, yes to changing the error index, no to changing standalone markdown files. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great - this is very helpful context, thanks for explaining. I agree with you. I'll update the code + maybe add a test for this standalone markdown feature if I can figure out how to do that. |
||
self.buf.push_back((Event::Html(format!("</a></h{}>", level).into()), 0..0)); | ||
|
||
let start_tags = format!( | ||
|
@@ -1005,7 +1010,7 @@ impl LangString { | |
|
||
impl Markdown<'_> { | ||
pub fn into_string(self) -> String { | ||
let Markdown(md, links, mut ids, codes, edition, playground) = self; | ||
let Markdown(md, links, mut ids, codes, edition, playground, level) = self; | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// This is actually common enough to special-case | ||
if md.is_empty() { | ||
|
@@ -1026,7 +1031,7 @@ impl Markdown<'_> { | |
|
||
let mut s = String::with_capacity(md.len() * 3 / 2); | ||
|
||
let p = HeadingLinks::new(p, None, &mut ids); | ||
let p = HeadingLinks::new(p, None, &mut ids, level); | ||
let p = Footnotes::new(p); | ||
let p = LinkReplacer::new(p.map(|(ev, _)| ev), links); | ||
let p = TableWrapper::new(p); | ||
|
@@ -1048,7 +1053,7 @@ impl MarkdownWithToc<'_> { | |
let mut toc = TocBuilder::new(); | ||
|
||
{ | ||
let p = HeadingLinks::new(p, Some(&mut toc), &mut ids); | ||
let p = HeadingLinks::new(p, Some(&mut toc), &mut ids, 0); | ||
let p = Footnotes::new(p); | ||
let p = TableWrapper::new(p.map(|(ev, _)| ev)); | ||
let p = CodeBlocks::new(p, codes, edition, playground); | ||
|
@@ -1077,7 +1082,7 @@ impl MarkdownHtml<'_> { | |
|
||
let mut s = String::with_capacity(md.len() * 3 / 2); | ||
|
||
let p = HeadingLinks::new(p, None, &mut ids); | ||
let p = HeadingLinks::new(p, None, &mut ids, 0); | ||
let p = Footnotes::new(p); | ||
let p = TableWrapper::new(p.map(|(ev, _)| ev)); | ||
let p = CodeBlocks::new(p, codes, edition, playground); | ||
|
@@ -1295,7 +1300,7 @@ crate fn markdown_links(md: &str) -> Vec<MarkdownLink> { | |
// There's no need to thread an IdMap through to here because | ||
// the IDs generated aren't going to be emitted anywhere. | ||
let mut ids = IdMap::new(); | ||
let iter = Footnotes::new(HeadingLinks::new(p, None, &mut ids)); | ||
let iter = Footnotes::new(HeadingLinks::new(p, None, &mut ids, 0)); | ||
|
||
for ev in iter { | ||
if let Event::Start(Tag::Link(kind, dest, _)) = ev.0 { | ||
|
Uh oh!
There was an error while loading. Please reload this page.