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

rustdoc: don't collapse docblock-short #36293

Merged
merged 2 commits into from
Sep 15, 2016
Merged

Conversation

liigo
Copy link
Contributor

@liigo liigo commented Sep 6, 2016

docblock-short

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

@frewsxcv
Copy link
Member

frewsxcv commented Sep 6, 2016


failures:

thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:295
---- [rustdoc] rustdoc/issue-32374.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "python" "/build/src/etc/htmldocck.py" "/build/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-32374.stage2-x86_64-unknown-linux-gnu" "/build/src/test/rustdoc/issue-32374.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
16: @has check failed
    `XPATH PATTERN` did not match
    // @has issue_32374/index.html '//*[@class="docblock short"]' '[Deprecated] [Unstable]'

Encountered 1 errors

------------------------------------------

thread '[rustdoc] rustdoc/issue-32374.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:2359
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [rustdoc] rustdoc/issue-32374.rs

test result: FAILED. 120 passed; 1 failed; 1 ignored; 0 measured

@steveklabnik
Copy link
Member

I like this, but yeah, seems like it fails the tests.

@bluss
Copy link
Member

bluss commented Sep 6, 2016

Some of the styling of docblock will no longer apply to the docblock-short lines since they change class. Does it matter? I think this one is needed, and maybe more?

.docblock {
    margin-left: 24px;
}

@liigo
Copy link
Contributor Author

liigo commented Sep 7, 2016

@steveklabnik I've fixed test failure, and rebased. r?

@steveklabnik
Copy link
Member

@liigo did you check on what @bluss was saying? I'm not sure if it matters or not.

@liigo
Copy link
Contributor Author

liigo commented Sep 8, 2016

@steveklabnik @bluss
I think, .docblock { margin-left: 24px; } is used for making room for the [−] button at the top-left corner of a docblock.

docblock-magin


If remove it, something will get messed:

docblock-magin-0


But for docblock-shorts, there're no [-] button before them, so no need for extra margin.

@bluss
Copy link
Member

bluss commented Sep 8, 2016

@liigo Aha, I tested locally and that seems right. Looks like .docblock code { } is missing though, so that rule needs to be updated.

@liigo
Copy link
Contributor Author

liigo commented Sep 8, 2016

I'll recheck. Thank you @bluss !

@liigo
Copy link
Contributor Author

liigo commented Sep 12, 2016

Done. Could you have another review? Thank you! @bluss

@steveklabnik
Copy link
Member

@bors: r+

thank you!

@bors
Copy link
Contributor

bors commented Sep 14, 2016

📌 Commit 26d5f99 has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Sep 14, 2016

⌛ Testing commit 26d5f99 with merge 6ffdda1...

@bors bors merged commit 26d5f99 into rust-lang:master Sep 15, 2016
@bluss
Copy link
Member

bluss commented Sep 16, 2016

This change looks great in real life 👍

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.

6 participants