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

Doc improvement on std::fmt module #33129

Merged
merged 1 commit into from
May 11, 2016
Merged

Conversation

GuillaumeGomez
Copy link
Member

Part of #29355.

r? @steveklabnik

@steveklabnik
Copy link
Member

failures:

---- fmt::write_0 stdout ----
    <anon>:7:5: 7:65 error: unused result which must be used
<anon>:7     fmt::write(&mut output, format_args!("Hello {}!", "world"));
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:1:9: 1:17 note: lint level defined here
<anon>:1 #![deny(warnings)]
                 ^~~~~~~~
error: aborting due to previous error(s)
thread 'fmt::write_0' panicked at 'Box<Any>', src/librustc/session/mod.rs:164
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- fmt::write_1 stdout ----
    <std macros>:2:1: 2:54 error: unused result which must be used
<std macros>:2 $ dst . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) )
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<std macros>:2:1: 2:54 note: in this expansion of write! (defined in <std macros>)
<anon>:1:9: 1:17 note: lint level defined here
<anon>:1 #![deny(warnings)]
                 ^~~~~~~~
error: aborting due to previous error(s)
thread 'fmt::write_1' panicked at 'Box<Any>', src/librustc/session/mod.rs:164


failures:
    fmt::write_0
    fmt::write_1

@GuillaumeGomez
Copy link
Member Author

Updated.

/// use std::fmt;
///
/// let mut output = String::new();
/// fmt::write(&mut output, format_args!("Hello {}!", "world")).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Could this maybe use try! instead? It will require wrapping it in a function, like most of the IO docs do.

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Apr 26, 2016

Choose a reason for hiding this comment

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

I didn't want to do it because of this. I think that adding a function (and more code) will lose the point I'm trying to demonstrate.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but unwrap() is not the idiomatic way to handle errors, and we don't want to show unidiomatic code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then I'll go with a full match pattern.

@GuillaumeGomez GuillaumeGomez force-pushed the fmt_doc branch 2 times, most recently from 08e19da to 93532fb Compare April 26, 2016 22:51
@GuillaumeGomez
Copy link
Member Author

Updated (I went for expect method finally).

@steveklabnik
Copy link
Member

I am still not sure that i prefer expect here, but we also have no official standards, so seems fine.

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Apr 29, 2016

📌 Commit 93532fb has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Apr 29, 2016

⌛ Testing commit 93532fb with merge 2f3d5c4...

@bors
Copy link
Contributor

bors commented Apr 29, 2016

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

@GuillaumeGomez
Copy link
Member Author

Updated.

@steveklabnik
Copy link
Member

@GuillaumeGomez did you push? I'm not seeing a new commit.

@GuillaumeGomez
Copy link
Member Author

I squashed it. I fixed the failed test.

@steveklabnik
Copy link
Member

Weird, github's UI is being very strange.

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 2, 2016

📌 Commit 0690720 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 2, 2016
@Manishearth
Copy link
Member

@bors r-

please don't commit libc changes. Don't use git add -a

Manishearth added a commit to Manishearth/rust that referenced this pull request May 8, 2016
bors added a commit that referenced this pull request May 8, 2016
Rollup of 9 pull requests

- Successful merges: #32900, #33129, #33365, #33383, #33474, #33478, #33480, #33484, #33493
- Failed merges: #33360
Manishearth added a commit to Manishearth/rust that referenced this pull request May 9, 2016
Manishearth added a commit to Manishearth/rust that referenced this pull request May 9, 2016
bors added a commit that referenced this pull request May 9, 2016
Rollup of 10 pull requests

- Successful merges: #33129, #33224, #33370, #33383, #33431, #33474, #33480, #33496, #33509, #33514
- Failed merges:
@Manishearth
Copy link
Member

Documenting stage2 error index (x86_64-unknown-linux-gnu)
Linkcheck stage2 (x86_64-unknown-linux-gnu)
collections/fmt/fn.write.html:79: broken link - collections/macro.write!.html
thread '<main>' panicked at 'found some broken links', /buildslave/rust-buildbot/slave/auto-linux-64-opt-rustbuild/build/src/tools/linkchecker/main.rs:54
note: Run with `RUST_BACKTRACE=1` for a backtrace.

@GuillaumeGomez
Copy link
Member Author

I checked the urls by hand. It's getting strange...

bors added a commit that referenced this pull request May 9, 2016
Rollup of 10 pull requests

- Successful merges: #33129, #33224, #33370, #33383, #33431, #33474, #33480, #33496, #33509, #33514
- Failed merges:
@GuillaumeGomez
Copy link
Member Author

@Manishearth: I just re-checked: both urls are correct.

@Manishearth
Copy link
Member

But does the linkcheck make step pass?

@mitaa
Copy link
Contributor

mitaa commented May 10, 2016

Note that the collections::fmt::write page fails but not std::fmt::write.

This is the case since write! is not in collections.

@GuillaumeGomez
Copy link
Member Author

@mitaa: Any clue about the guilty url?

@mitaa
Copy link
Contributor

mitaa commented May 10, 2016

Maybe you could always link to the write! of std or core?
/// [write_macro]: ../../std/macro.write!.html

@GuillaumeGomez
Copy link
Member Author

I'll try it. Thanks!

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 10, 2016
@steveklabnik
Copy link
Member

@bors: r-

@GuillaumeGomez
Copy link
Member Author

And updated again.

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented May 11, 2016

📌 Commit 0908d66 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 11, 2016
bors added a commit that referenced this pull request May 11, 2016
Rollup of 9 pull requests

- Successful merges: #33129, #33260, #33345, #33386, #33522, #33524, #33528, #33539, #33542
- Failed merges: #33342, #33475, #33517
@bors bors merged commit 0908d66 into rust-lang:master May 11, 2016
@GuillaumeGomez GuillaumeGomez deleted the fmt_doc branch May 17, 2016 11:19
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.

5 participants