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

Using X headings #39850 #40496

Merged
merged 1 commit into from
Mar 17, 2017
Merged

Using X headings #39850 #40496

merged 1 commit into from
Mar 17, 2017

Conversation

projektir
Copy link
Contributor

Fix for issue #39850, the headings should now be 1, 2, and 3.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @frewsxcv (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@frewsxcv
Copy link
Member

Thanks! @bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 13, 2017

📌 Commit 11d3344 has been approved by frewsxcv

@steveklabnik
Copy link
Member

This is a big bummer because now we get multiple H1s on the page.

@frewsxcv
Copy link
Member

I don't think there's anything wrong with having multiple <h1>s on a page. The HTML spec even has an example consisting solely of nested <h1> headings:

https://html.spec.whatwg.org/multipage/semantics.html#headings-and-sections

@frewsxcv
Copy link
Member

frewsxcv commented Mar 14, 2017

Though, after reading again, that's probably because they're using <section>s to indicate sub-headings. Regardless, I'm still in favor of these changes since I don't see how a series of adjacent <h1>s is semantically any worse than a series of adjacent parent-less <h2>s.

@steveklabnik
Copy link
Member

steveklabnik commented Mar 14, 2017 via email

@projektir
Copy link
Contributor Author

@steveklabnik the Grammar page seems to be doing the same thing. But if you think this is a poor change, it seems that at least one h1 heading is needed or the 0.x issue will occur. This seems to be an effect coming from mdBook (?), maybe if this was treated as just a straight up Markdown page this wouldn't happen?

I'm happy to undo this or research further to fix this properly.

@steveklabnik
Copy link
Member

This page is rendered by rustdoc, not mdbook. It might be rust.css, I'm not sure.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 14, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 14, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
@projektir
Copy link
Contributor Author

projektir commented Mar 14, 2017

I've looked at rustdoc, and the numbering seems to be managed by the ToC generator and the 0.x even looks like expected behavior. Perhaps it can be modified to treat the % title as a parent, since that will become an <h1>.

I will note that, from the looks of it, if you have an X in your Markdown at all, you're going to have multiple <h1>'s, since there's always at least one <h1> for the % title. Perhaps X's should become <h2>'s and so on, but I don't know how you guys typically use Markdown.

I'll look at it and see if I can figure something out and undo this change as well as fix the grammar page.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 15, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
@steveklabnik
Copy link
Member

Perhaps it can be modified to treat the % title as a parent, since that will become an

.

I really hate that the % is even a thing. So much work to do with rustdoc, ugh.

bors added a commit that referenced this pull request Mar 15, 2017
bors added a commit that referenced this pull request Mar 16, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 17, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 17, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 17, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 17, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 17, 2017
…rewsxcv

Using X headings rust-lang#39850

Fix for issue rust-lang#39850, the headings should now be 1, 2, and 3.
bors added a commit that referenced this pull request Mar 17, 2017
@bors bors merged commit 11d3344 into rust-lang:master Mar 17, 2017
@projektir projektir deleted the docs_number_headings branch March 18, 2017 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants