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.

Sign up for GitHub

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

Jump to bottom

Fix issues with Markdown summary options #88632

Merged
merged 2 commits into from
Sep 11, 2021
Merged

Fix issues with Markdown summary options #88632

merged 2 commits into from
Sep 11, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Sep 4, 2021

  • Use summary_opts() for Markdown summaries
  • Enable all main body Markdown options for summaries

@camelid
Use summary_opts() for Markdown summaries
208a5fd 
It was accidentally changed to use `opts()` in rust-lang#86451.

I also renamed `opts()` to `main_body_opts()` to make this kind of
accidental change less likely.
@camelid
Enable all main body Markdown options for summaries
2cc7b7c 
This fixes odd renderings when these features are used in the first
paragraph of documentation for an item. This is an extension of rust-lang#87270.
@camelid camelid added A-markdown-parsing Area: Markdown parsing for doc-comments T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 4, 2021
@rust-highfive
Copy link
Collaborator

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 4, 2021
@camelid
Copy link
Member Author

camelid commented Sep 4, 2021

(There are more details about the changes in the commit messages.)

@CraftSpider
Copy link
Contributor

Does a tasklist actually look okay in a markdown summary? I guess it doesn't necessarily matter, not rendering it probably looks worse, but I'm curious what kind of affects this has.

@camelid
Copy link
Member Author

camelid commented Sep 8, 2021

Does a tasklist actually look okay in a markdown summary? I guess it doesn't necessarily matter, not rendering it probably looks worse, but I'm curious what kind of affects this has.

It doesn't actually get rendered, because markdown_summary_with_limit stops as soon as it encounters things like task lists. I've tested it locally, though I don't have a screenshot available since it was a few days ago, but the difference is like this:

input

- [ ] foo
- [ ] bar

output before

(note that this is shown literally as the characters - [ ] foo)

- [ ] foo

output after

(nothing)

@CraftSpider
Copy link
Contributor

Alright, thanks for the clarification. Definitely sounds like better behavior now.
@bors r+

@bors
Copy link
Contributor

bors commented Sep 8, 2021

📌 Commit 2cc7b7c has been approved by CraftSpider

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 9, 2021
@GuillaumeGomez
Rollup merge of rust-lang#88632 - camelid:md-opts, r=CraftSpider
ff0f5f9 
Fix issues with Markdown summary options

- Use `summary_opts()` for Markdown summaries
- Enable all main body Markdown options for summaries
@GuillaumeGomez GuillaumeGomez mentioned this pull request Sep 9, 2021
Closed
@Manishearth Manishearth mentioned this pull request Sep 10, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 11, 2021
@bors
Auto merge of rust-lang#88824 - Manishearth:rollup-7bzk9h6, r=Manishe…
22719ef 
…arth

Rollup of 15 pull requests

Successful merges:

 - rust-lang#85200 (Ignore derived Clone and Debug implementations during dead code analysis)
 - rust-lang#86165 (Add proc_macro::Span::{before, after}.)
 - rust-lang#87088 (Fix stray notes when the source code is not available)
 - rust-lang#87441 (Emit suggestion when passing byte literal to format macro)
 - rust-lang#88546 (Emit proper errors when on missing closure braces)
 - rust-lang#88578 (fix(rustc): suggest `items` be borrowed in `for i in items[x..]`)
 - rust-lang#88632 (Fix issues with Markdown summary options)
 - rust-lang#88639 (rustdoc: Fix ICE with `doc(hidden)` on tuple variant fields)
 - rust-lang#88667 (Tweak `write_fmt` doc.)
 - rust-lang#88720 (Rustdoc coverage fields count)
 - rust-lang#88732 (RustWrapper: avoid deleted unclear attribute methods)
 - rust-lang#88742 (Fix table in docblocks)
 - rust-lang#88776 (Workaround blink/chromium grid layout limitation of 1000 rows)
 - rust-lang#88807 (Fix typo in docs for iterators)
 - rust-lang#88812 (Fix typo `option` -> `options`.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1043549 into rust-lang:master Sep 11, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 11, 2021
@camelid camelid deleted the md-opts branch September 11, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@CraftSpider CraftSpider

Labels
A-markdown-parsing Area: Markdown parsing for doc-comments S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.57.0
Development

Successfully merging this pull request may close these issues.
5 participants
@camelid @rust-highfive @CraftSpider @bors @rustbot