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

Merge guidelines from RFC 1567 into UX Guidelines. #34894

Closed
wants to merge 1 commit into from

Conversation

Havvy
Copy link
Contributor

@Havvy Havvy commented Jul 18, 2016

This is a partial fix for issue #34808.

Most of the wording was copied verbatim from the RFC. Main difference is that I moved the actual template to the top of the section.

It also makes the error explanations the longest section in the guidelines doc for now.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

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

@steveklabnik
Copy link
Member

This looks decent to me but I don't have time to do a full review right now, if anyone else feels like it's good, feel free to r+

@steveklabnik
Copy link
Member

@bors: r+ rollup

Thanks! :)

@bors
Copy link
Contributor

bors commented Jul 25, 2016

📌 Commit 010e479 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 25, 2016
Merge guidelines from RFC 1567 into UX Guidelines.

This is a partial fix for issue rust-lang#34808.

Most of the wording was copied verbatim from the RFC. Main difference is that I moved the actual template to the top of the section.

It also makes the error explanations the longest section in the guidelines doc for now.
bors added a commit that referenced this pull request Jul 25, 2016
Rollup of 13 pull requests

- Successful merges: #34461, #34609, #34732, #34850, #34894, #34935, #34974, #34990, #34995, #35001, #35009, #35010, #35028
- Failed merges:
@bors
Copy link
Contributor

bors commented Jul 26, 2016

⌛ Testing commit 010e479 with merge cda0675...

@bors
Copy link
Contributor

bors commented Jul 26, 2016

💔 Test failed - auto-win-msvc-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Tue, Jul 26, 2016 at 4:16 AM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-opt
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt/builds/5088


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#34894 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95Csr5A8xmi0Bn-fQ5N36aIPJZuc5ks5qZeyjgaJpZM4JOWxQ
.

@steveklabnik
Copy link
Member

\```

[Optional Additional information]
```
Copy link
Member

Choose a reason for hiding this comment

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

looks like this wasn't closed here

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 27, 2016

📌 Commit f1a51bd has been approved by steveklabnik

@GuillaumeGomez
Copy link
Member

@bors: r-

It definitely makes windows build fail. You have to add ignore flag on every code block.

bors added a commit that referenced this pull request Jul 29, 2016
Rollup of 7 pull requests

- Successful merges: #34258, #34894, #35050, #35062, #35066, #35072, #35087
- Failed merges:
@Havvy
Copy link
Contributor Author

Havvy commented Jul 30, 2016

Actually, this shows a bigger issue. I want to show the example and the template as raw text, not as markdown. But I don't know how to do that...I was hoping the blockquotes did it, but it doesn't. It looks like the backslash from the template also gets put into the output.

@steveklabnik
Copy link
Member

use

```text

@Havvy
Copy link
Contributor Author

Havvy commented Jul 30, 2016

Using ```text doesn't work here, because it stops at the first lone end code section it encounters.

You can see that in the current document.

I need some way of nesting code blocks.

This is a partial fix for issue rust-lang#34808.

Most of the wording was copied verbatim from the RFC. Main difference is that I moved the actual template to the top of the section.

It also makes the error explanations the longest section in the guidelines doc for now.
@Havvy
Copy link
Contributor Author

Havvy commented Aug 1, 2016

I think this will work. Rebased my messing around with markdown into one commit. Also fixed up some of the attempts of escaping markdown from the original template in the RFC.

I hope everything is good now?

@GuillaumeGomez
Copy link
Member

Seems good for me.

cc @steveklabnik

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Aug 3, 2016

📌 Commit cede35e has been approved by steveklabnik

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Aug 4, 2016
Merge guidelines from RFC 1567 into UX Guidelines.

This is a partial fix for issue rust-lang#34808.

Most of the wording was copied verbatim from the RFC. Main difference is that I moved the actual template to the top of the section.

It also makes the error explanations the longest section in the guidelines doc for now.
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Aug 4, 2016
Merge guidelines from RFC 1567 into UX Guidelines.

This is a partial fix for issue rust-lang#34808.

Most of the wording was copied verbatim from the RFC. Main difference is that I moved the actual template to the top of the section.

It also makes the error explanations the longest section in the guidelines doc for now.
@GuillaumeGomez
Copy link
Member

It still doesn't work:

doc tests for: C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src/doc\rustc-ux-guidelines.md

running 6 tests
test _1 ... ignored
test _2 ... ignored
test _3 ... ignored
test _4 ... ignored
test _5 ... FAILED
test _0 ... FAILED

failures:

---- _5 stdout ----
    <anon>:2:12: 41:4 error: expected type, found `r##"
An "or" pattern was used where the variable bindings are not consistently bound
across patterns.

Example of erroneous code:

```compile_fail,E0409
let x = (0, 2);
match x {
    (0, ref y) | (y, 0) ={ /* use y */} // error: variable `y` is bound with
                                          //        different mode in pattern #2
                                          //        than in pattern #1
    _ =()
}

Here, y is bound by-value in one case and by-reference in the other.

To fix this error, just use the same mode in both cases.
Generally using ref or ref mut where not already used will fix this:

let x = (0, 2);
match x {
    (0, ref y) | (ref y, 0) ={ /* use y */}
    _ =()
}

Alternatively, split the pattern:

let x = (0, 2);
match x {
    (y, 0) ={ /* use y */ }
    (0, ref y) ={ /* use y */}
    _ =()
}

"##`
:2 E0409: r##"
^
error: aborting due to previous error(s)
thread '_5' panicked at 'Box', C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src\librustc\session/mod.rs:168

---- _0 stdout ----
:2:11: 19:4 error: expected type, found `r##"
[Error description]

Example of erroneous code:

[Minimal example]

[Error explanation]

[How to fix the problem]

[Optional Additional information]

"## <anon>:2 E000: r##" ^ error: aborting due to previous error(s) thread '_0' panicked at 'Box<Any>', C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src\librustc\session/mod.rs:168 note: Run withRUST_BACKTRACE=1` for a backtrace.

failures:
_0
_5

test result: FAILED. 0 passed; 2 failed; 4 ignored; 0 measured

command did not execute successfully: "C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\obj\build\i686-pc-windows-gnu\stage2\bin\rustdoc.exe" "--test" "C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src/doc\rustc-ux-guidelines.md" "--test-args" ""
expected success, got: exit code: 101

@GuillaumeGomez
Copy link
Member

@bors: r-

bors added a commit that referenced this pull request Aug 5, 2016
@steveklabnik
Copy link
Member

@Havvy ping! Still keeping this PR going?

@Havvy
Copy link
Contributor Author

Havvy commented Aug 15, 2016

I'm stuck at the moment because I'm not sure how to show markdown that includes code blocks without putting ignore on them. I was going to see if I could mess with the test finder code to see what's going on. And then other duties took over while I was looking into it.

@Havvy
Copy link
Contributor Author

Havvy commented Aug 18, 2016

Closing this until I have more time to look at it.

@Havvy Havvy closed this Aug 18, 2016
@Havvy Havvy deleted the patch-2 branch September 27, 2017 00: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.

6 participants