Skip to content

Add a write_char method to std::fmt::Write #24661

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 3 commits into from
Apr 22, 2015

Conversation

SimonSapin
Copy link
Contributor

as accepted in RFC 526.

Note that this brand new method is marked as stable. I judged this safe enough: it’s simple enough that it’s very unlikely to change. Still, I can mark it unstable instead if you prefer.

r? @alexcrichton

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

@aturon
Copy link
Member

aturon commented Apr 21, 2015

I'm fine with this coming in as #[stable] (as you say, it's very unlikely to change).

That said, I don't think we'll cherry-pick this for 1.0, so can you please put since = "1.1.0"?

@SimonSapin
Copy link
Contributor Author

Done. Is it still feature = "rust1"?

@aturon
Copy link
Member

aturon commented Apr 21, 2015

@bors: r+

@bors
Copy link
Collaborator

bors commented Apr 21, 2015

📌 Commit 16181e6 has been approved by aturon

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 21, 2015
as accepted in [RFC 526](https://github.com/rust-lang/rfcs/blob/master/text/0526-fmt-text-writer.md).

Note that this brand new method is marked as **stable**. I judged this safe enough: it’s simple enough that it’s very unlikely to change. Still, I can mark it unstable instead if you prefer.

r? @alexcrichton
@Valloric
Copy link
Contributor

No tests? :(

@ncm
Copy link

ncm commented Apr 22, 2015

I am kind of out of the loop, here. Is Rust perpetuating C's confusion of octets with characters? Or does write_char write out enough octets to form a code point (which we can, informally, call a character because it's close enough for most purposes)? If it just writes a byte, that seems like a worthy goal, but then the name is wrong.

@SimonSapin
Copy link
Contributor Author

@ncm: I don’t think there is such a confusion. A char is a Unicode scalar value. The point of std::fmt::Write (as opposed to std::io::Write) is to only accept well-formed Unicode data, not arbitrary bytes.

My goal here is to use this trait not just for formatting, but as a general-purpose Unicode-based stream. The default implementation of write_char is to write up to four bytes with write_str, but it could be the reverse (write_char is fundamental and write_str uses it) in an implementation that’s not based on UTF-8.

@bors bors merged commit 16181e6 into rust-lang:master Apr 22, 2015
@SimonSapin SimonSapin deleted the fmt-write-char branch April 22, 2015 06:56
@SimonSapin
Copy link
Contributor Author

@Valloric My bad. #24688 adds a test.

SimonSapin added a commit to SimonSapin/rust that referenced this pull request Apr 22, 2015
This is the logical next step after rust-lang#24661, but I’m less sure about this one.
Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 22, 2015
SimonSapin added a commit to SimonSapin/rust that referenced this pull request Jun 10, 2015
This is the logical next step after rust-lang#24661, but I’m less sure about this one.
bors added a commit that referenced this pull request Jun 10, 2015
This is the logical next step after #24661, but I’m less sure about this one.

r? @alexcrichton
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.

7 participants