rustdoc: Return String from doc_value, not Option #92079
Conversation
`render/context` always runs after `run_global_context`, so it was always set to `true`. This is a holdover from when rustdoc allowed configuring passes, but the `collapse-docs` pass was removed ages ago, and the ability to configure passes is about to be removed.
Very few places actually need to special case this, and those that do can use `.is_empty()` instead. Note that both `plain_text_summary` and `short_markdown_summary()` already special case empty strings, so this is still taking the fast path for those docs.
jyn514
added
C-cleanup
|
Some changes occurred in cc @camelid |
rust-highfive
added
the
S-waiting-on-review
jyn514
changed the title
|
Hum actually I'm really not sure it's the right way to do so... I explained why here. |
GuillaumeGomez
dismissed
their stale review
Not sure if it's the right solution
camelid
added
S-blocked
| @@ -525,10 +525,11 @@ fn document_short( | |||
| if !show_def_docs { | |||
| return; | |||
| } | |||
| if let Some(s) = item.doc_value() { | |||
| let mut summary_html = MarkdownSummaryLine(&s, &item.links(cx)).into_string(); | |||
| let dox = &item.doc_value(); | |||
There was a problem hiding this comment.
| let dox = &item.doc_value(); | |
| let dox = item.doc_value(); |
There was a problem hiding this comment.
How does this even compile currently? Doesn't it borrow a temporary?
There was a problem hiding this comment.
Your suggestion requires borrowing at all the use sites, which makes it more noisy IMO.
The reference here behaves the same as
let dox = item.doc_value()
let dox = &dox;
I don't know exactly the rule for it, just that it works.
There was a problem hiding this comment.
Your suggestion requires borrowing at all the use sites, which makes it more noisy IMO.
I only see one use site that would need a borrow?
jyn514
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #92095) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I don't have energy to argue about any of this. Feel free to make your own PR if you think it's useful. |
Very few places actually need to special case this, and those that do can use
.is_empty()instead.Note that both
plain_text_summaryandshort_markdown_summary()already special case emptystrings, so this is still taking the fast path for those docs.
This is similar to #92078, but for
doc_valueinstead ofcollapsed_doc_value. Builds on #92077.r? @camelid cc @GuillaumeGomez #91072 (comment)