Skip to content

rustdoc: fix summary lines #31902

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

Merged
merged 2 commits into from
Feb 28, 2016
Merged

rustdoc: fix summary lines #31902

merged 2 commits into from
Feb 28, 2016

Conversation

mitaa
Copy link
Contributor

@mitaa mitaa commented Feb 26, 2016

@mitaa
Copy link
Contributor Author

mitaa commented Feb 26, 2016

cc @bluss

@bluss
Copy link
Member

bluss commented Feb 26, 2016

Response time is A+++ 😄

@mitaa
Copy link
Contributor Author

mitaa commented Feb 26, 2016

I just remembered that this function is also used for the search-index descriptions, which are not markdown rendered. So that still needs the old behaviour.

I'm going to push an update as soon as I have time.

@alexcrichton
Copy link
Member

The idea here was to render headers like:

/// really long intro
/// comment
///
/// more explanation

as "really long intro comment", but doesn't this break that as only the first line is taken?

@mitaa mitaa force-pushed the rdoc-shorter branch 5 times, most recently from dfaeaf8 to 5d33b35 Compare February 26, 2016 16:45
@mitaa
Copy link
Contributor Author

mitaa commented Feb 26, 2016

You're right, I guess I got a bit confused by looking at the HTML source through inspector, which doesn't show the newlines in the <p> element (I thought hoedown/markdown removes these).
The rendererd markdown for

/// A tuple or fixed size array that can be used to index an array.
/// Make this line a bit longer.
///
/// ```
/// use ndarray::arr2;
/// ```
pub fn bar() {}

actually is

<p>A tuple or fixed size array that can be used to index an array.
Make this line a bit longer.</p>
<pre class='rust rust-example-rendered'>
<span class='kw'>use</span> <span class='ident'>ndarray</span>::<span class='ident'>arr2</span>;</pre>

Because of this I'm now extracting the first occuring html element in the string which seems to me the correct way to go.
I don't think that there is something in-tree with which I could replace the parsing code I've written?

@alexcrichton
Copy link
Member

Er hang on, can we drill into what the bug is in the first place? I'd really like to avoid having any sort of HTML parser in rustdoc just to extract the first <p> span or something like that, it sounds quite brittle!

It looks like the bug may be that we're extracting text after rendering, when we should be extracting before rendering?

@mitaa
Copy link
Contributor Author

mitaa commented Feb 26, 2016

I'm a bit uneasy about the parsing, but extracting before rendering means reintroducing #30366 and #25787.

@mitaa
Copy link
Contributor Author

mitaa commented Feb 26, 2016

The problem doesnt appear with two paragraps, but with one paragraph and a codeblock. Specifically hoedown doesnt leave a blank line in this case.

..perhaps we could insert a dummy item between the first and second block, which we know leaves a blank line, before we markdown render it? (and then shorten it after rendering)

/// A tuple or fixed size array that can be used to index an array.
/// Make this line a bit longer.
///
/// # dummy
/// ```
/// use ndarray::arr2;
/// ``````

@alexcrichton
Copy link
Member

Can we perhaps alter the hoedown rendering of code blocks to insert the blank line for us? That may be a good fix in the case that code blocks are the only cause of this.

@mitaa
Copy link
Contributor Author

mitaa commented Feb 27, 2016

Oh right, we can do that 😄

(updated)

For summary descriptions we need the first paragraph (adjacent lines
until a blank line) - but the rendered markdown of a code block did not
leave a blank line in the html and was thus included in the summary line.
For plaintext we don't actually need to render the Markdown before
shortening the string. (and this may have led to wrong output)
@alexcrichton
Copy link
Member

@bors: r+ 0b3bc9b

Thanks!

bors added a commit that referenced this pull request Feb 28, 2016
@bors
Copy link
Collaborator

bors commented Feb 28, 2016

⌛ Testing commit 0b3bc9b with merge 095f5e7...

@bors bors merged commit 0b3bc9b into rust-lang:master Feb 28, 2016
@bluss
Copy link
Member

bluss commented Feb 28, 2016

Thank you @mitaa !

@mitaa mitaa deleted the rdoc-shorter branch February 28, 2016 14:43
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.

rustdoc summaries include parts of doc examples
4 participants