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

print enum variant fields in docs #33867

Merged
merged 1 commit into from
May 30, 2016
Merged

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 25, 2016

Right now we are repeating enum variants at the top, because the fields aren't shown with the actual docs. It's very annoying to have to scroll up and down to have both docs and field info. For struct variants we already list the fields.

enum docs look like this after this PR:

screenshot from 2016-05-25 14-02-42

There are degenerate cases for enum tuple variants with lots of fields:

screenshot from 2016-05-25 14-01-00

I was thinking that we could move the docs below the variant (slightly indented) or list the variant fields vertically instead of horizontally

r? @steveklabnik

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 25, 2016

I remember that someone talked about it (or maybe even open a PR?). Could you also provide a screenshot for your change please? I like to see wonderful changes rendered. 😃

@oli-obk
Copy link
Contributor Author

oli-obk commented May 25, 2016

These are the screenshots after my changes.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 25, 2016

Oh, my bad. Could you align description text with the variant name please? (and screenshot with the change as well please 😛 )

@oli-obk
Copy link
Contributor Author

oli-obk commented May 25, 2016

@GuillaumeGomez
Copy link
Member

@oli-obk: For the overlap issue once collapsed:

.enum > .collapsed, .struct > .collapsed {
    margin-bottom: 25px;
}

@oli-obk
Copy link
Contributor Author

oli-obk commented May 27, 2016

updated (live version updated, too)

@oli-obk
Copy link
Contributor Author

oli-obk commented May 27, 2016

travis failure is an http-failure during apt-get

@oli-obk
Copy link
Contributor Author

oli-obk commented May 27, 2016

do you want me to remove the (now useless) enum/struct definition at the top?

@GuillaumeGomez
Copy link
Member

If it's not used anymore, then yes please.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 27, 2016

One last thing. You need to change:

.enum > .toggle-wrapper + .docblock, .struct > .toggle-wrapper + .docblock {
    margin-left: 10px;
    margin-bottom: 20px;
    margin-top: 5px;
}

to:

.enum > .toggle-wrapper + .docblock, .struct > .toggle-wrapper + .docblock {
    margin-left: 30px;
    margin-bottom: 20px;
    margin-top: 5px;
}

@oli-obk oli-obk force-pushed the rustdoc_variant_types branch from 00cd95e to 0dc0d1f Compare May 27, 2016 09:52
@oli-obk
Copy link
Contributor Author

oli-obk commented May 27, 2016

added changes and squashed commits

@@ -591,13 +610,11 @@ a.test-arrow {
.collapse-toggle {
font-weight: 300;
position: absolute;
left: -23px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so this rule shouldn't be removed.

@GuillaumeGomez
Copy link
Member

You need to move the css changes after this block:

span.since {
    position: initial;
    font-size: 20px;
    margin-right: 5px;
}

@oli-obk oli-obk force-pushed the rustdoc_variant_types branch from 0dc0d1f to 94c3679 Compare May 27, 2016 10:28
@oli-obk
Copy link
Contributor Author

oli-obk commented May 27, 2016

updated and live version now looks good

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented May 27, 2016

Travis doesn't seem to be able to run apt-get. Strange... I'll put it in rollup and we'll see.

Thanks @oli-obk!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 27, 2016

📌 Commit 94c3679 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 29, 2016

⌛ Testing commit 94c3679 with merge 56ae85b...

@bors
Copy link
Contributor

bors commented May 29, 2016

💔 Test failed - auto-linux-64-opt-rustbuild

@GuillaumeGomez
Copy link
Member

A lot of doc tests are broken.

@bors: rollup-

@oli-obk
Copy link
Contributor Author

oli-obk commented May 30, 2016

I don't grok those error messages. How do I go about debugging them?

failures:

---- [rustdoc] rustdoc/impl-parts.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python2.7" "/home/ws/ca8159/Projects/rust/rust/src/etc/htmldocck.py" "x86_64-unknown-linux-gnu/test/rustdoc/impl-parts.stage1-x86_64-unknown-linux-gnu" "/home/ws/ca8159/Projects/rust/rust/src/test/rustdoc/impl-parts.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
19: @has check failed
    `XPATH PATTERN` did not match
    // @has impl_parts/struct.Foo.html '//*[@class="impl"]//code' "impl<T: Clone> !AnOibit for Foo<T> where T: Sync"
21: @has check failed
    `XPATH PATTERN` did not match
    // @has impl_parts/trait.AnOibit.html '//*[@class="item-list"]//code' "impl<T: Clone> !AnOibit for Foo<T> where T: Sync"

Encountered 2 errors

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

thread '[rustdoc] rustdoc/impl-parts.rs' panicked at 'explicit panic', /home/ws/ca8159/Projects/rust/rust/src/tools/compiletest/src/runtest.rs:2231
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- [rustdoc] rustdoc/issue-20727-2.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python2.7" "/home/ws/ca8159/Projects/rust/rust/src/etc/htmldocck.py" "x86_64-unknown-linux-gnu/test/rustdoc/issue-20727-2.stage1-x86_64-unknown-linux-gnu" "/home/ws/ca8159/Projects/rust/rust/src/test/rustdoc/issue-20727-2.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
18: @has check failed
    `XPATH PATTERN` did not match
        // @has - '//*[@class="rust trait"]' 'trait Add<RHS = Self> {'
28: @has check failed
    `XPATH PATTERN` did not match
        // @has - '//*[@class="rust trait"]' 'trait Add<RHS = Self> {'

Encountered 2 errors

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

thread '[rustdoc] rustdoc/issue-20727-2.rs' panicked at 'explicit panic', /home/ws/ca8159/Projects/rust/rust/src/tools/compiletest/src/runtest.rs:2231

---- [rustdoc] rustdoc/issue-20727-4.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python2.7" "/home/ws/ca8159/Projects/rust/rust/src/etc/htmldocck.py" "x86_64-unknown-linux-gnu/test/rustdoc/issue-20727-4.stage1-x86_64-unknown-linux-gnu" "/home/ws/ca8159/Projects/rust/rust/src/test/rustdoc/issue-20727-4.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
18: @has check failed
    `XPATH PATTERN` did not match
        // @has - '//*[@class="rust trait"]' 'trait Index<Idx: ?Sized> {'
29: @has check failed
    `XPATH PATTERN` did not match
        // @has - '//*[@class="rust trait"]' 'trait IndexMut<Idx: ?Sized>: Index<Idx> {'

Encountered 2 errors

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

thread '[rustdoc] rustdoc/issue-20727-4.rs' panicked at 'explicit panic', /home/ws/ca8159/Projects/rust/rust/src/tools/compiletest/src/runtest.rs:2231


failures:
    [rustdoc] rustdoc/impl-parts.rs
    [rustdoc] rustdoc/issue-20727-2.rs
    [rustdoc] rustdoc/issue-20727-4.rs

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

@GuillaumeGomez
Copy link
Member

It means the generated docs don't correspond to what tests expected. A small explanation of how it works is provided in "/home/ws/ca8159/Projects/rust/rust/src/etc/htmldocck.py".

@oli-obk
Copy link
Contributor Author

oli-obk commented May 30, 2016

Found the culprit. The python script treated &nbsp; as something other than whitespace during matching.

@GuillaumeGomez
Copy link
Member

Great, let's see what travis says and it'll be good to get merged.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 30, 2016

travis likes it

@GuillaumeGomez
Copy link
Member

Then here we go!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 30, 2016

📌 Commit 39a5f3c has been approved by GuillaumeGomez

@GuillaumeGomez
Copy link
Member

@bors: r-

Sorry, forgot to ask: can you squash your commits please?

@oli-obk oli-obk force-pushed the rustdoc_variant_types branch from 39a5f3c to b0c7033 Compare May 30, 2016 14:12
@oli-obk
Copy link
Contributor Author

oli-obk commented May 30, 2016

done

@GuillaumeGomez
Copy link
Member

Ok, now it's all good. Sorry about that.

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 30, 2016

📌 Commit b0c7033 has been approved by GuillaumeGomez

Manishearth added a commit to Manishearth/rust that referenced this pull request May 30, 2016
…illaumeGomez

print enum variant fields in docs

Right now we are repeating enum variants at the top, because the fields aren't shown with the actual docs. It's very annoying to have to scroll up and down to have both docs and field info. For struct variants we already list the fields.

enum docs look like this after this PR:

![screenshot from 2016-05-25 14-02-42](https://cloud.githubusercontent.com/assets/332036/15539231/84b018cc-2281-11e6-9666-1063655931f4.png)

There are degenerate cases for enum tuple variants with lots of fields:

![screenshot from 2016-05-25 14-01-00](https://cloud.githubusercontent.com/assets/332036/15539260/91e537ca-2281-11e6-8bf1-a3d6b2e78f65.png)

I was thinking that we could move the docs below the variant (slightly indented) or list the variant fields vertically instead of horizontally

r? @steveklabnik
bors added a commit that referenced this pull request May 30, 2016
Rollup of 5 pull requests

- Successful merges: #33867, #33926, #33942, #33958, #33964
- Failed merges:
@bors bors merged commit b0c7033 into rust-lang:master May 30, 2016
@oli-obk oli-obk deleted the rustdoc_variant_types branch June 2, 2016 11:10
@sanxiyn sanxiyn mentioned this pull request Jun 13, 2016
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.

4 participants