Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
librustdoc: Use correct heading levels. #89506
librustdoc: Use correct heading levels. #89506
Changes from 1 commit
a8a40ea
4a6aa6e
6518a0a
13558ee
f1425c7
08a4f24
742d8be
1f86a8e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
level = 0
, you get# My Header
➡️<h2>My Header</h2>
. This seems weird to me.I would have expected that
HeadingLinks { level: 0 }
would mean that headings aren't getting modified. Instead, you've made it so that there is no way to tell HeadingLinks to not bump the levels. It's also silently changing how the standalone markdown files (stuff like/src/doc/index.md
) are generated, and it's why the error index got changed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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 comment
The 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 comment
The 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 (
# this is an h1
) from doc comments in std (# this is an h2
)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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
.md
files, so you expect them to be viewed using things like GitHub's markdown preview pane, or inside an IDE). But when they're combined to form the full error index, the resulting page has multiple h1's.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 comment
The 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 comment
The 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.