Add a write_char method to std::fmt::Formatter. #24689

SimonSapin · Apr 22, 2015

This is the logical next step after #24661, but I’m less sure about this one.

rust-highfive · Apr 22, 2015

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.

alexcrichton · Apr 28, 2015

Hm can you clarify why you'd want this as an inherent method? It seems like a similar win is gained from implementing fmt::Write. The main reason fmt::Write isn't leveraged for write_fmt is that it would require an import of a somewhat obscure trait in lots of code, but I think that write_char is pretty rare by comparison.

SimonSapin · Apr 28, 2015

I only made it an inherent method because write_str was as well, and this seemed like the smaller change. Note that Formatter currently does not implement any trait.

I don’t have a strong opinion on how to do this, or if it should be done at all.

alexcrichton · Apr 28, 2015

I think it would be safe to provide an implementation of the trait for Formatter, but I think we may want to hold off adding a method for now.

alexcrichton · Jun 10, 2015

@SimonSapin could you try adding this method through an implementation of the Write trait on the Formatter struct directly? I think it should have the implementation anyway and seems like a conservative way to expose the functionality.

SimonSapin · Jun 10, 2015

Done. (Now the trait has to be in scope to use that method, of course.)

alexcrichton · Jun 10, 2015

alexcrichton reviewed Jun 10, 2015

View changes

alexcrichton · Jun 10, 2015

src/libcore/fmt/mod.rs

@@ -892,6 +892,20 @@ impl<'a> Formatter<'a> {
    }
 }

+impl<'a> Write for Formatter<'a> {


Could you also tag this with #[stable(since = "1.2.0", ...)]?

alexcrichton · Jun 10, 2015

r=me with a stability tag, thanks @SimonSapin!

alexcrichton · Jun 10, 2015

@bors: r+ 63da18b

bors · Jun 10, 2015

⌛️ Testing commit 63da18b with merge db0c1cb...

bors · Jun 10, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt

SimonSapin · Jun 29, 2015

SimonSapin reviewed Jun 29, 2015

View changes

SimonSapin · Jun 29, 2015

src/libcore/fmt/mod.rs

-        let mut utf8 = [0; 4];
-        let amt = self.encode_utf8(&mut utf8).unwrap_or(0);
-        let s: &str = unsafe { mem::transmute(&utf8[..amt]) };
-        Display::fmt(s, f)


Ah, yes, my mistake. Feel free to reverse this chunk if there isn’t a better solution.

rust-highfive assigned alexcrichton Apr 22, 2015

alexcrichton added T-libs I-needs-decision labels May 26, 2015

alexcrichton removed the I-needs-decision label Jun 10, 2015

Add a write_char method to std::fmt::Formatter.
This is the logical next step after #24661, but I’m less sure about this one.

15b028c

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from b3c46d0 to 10fbce3 Jun 10, 2015

Have std::fmt::Formatter implement std::fmt::Write.

Loading status checks…

63da18b

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from 10fbce3 to 63da18b Jun 10, 2015

bors added a commit that referenced this pull request Jun 10, 2015

Auto merge of #24689 - SimonSapin:formatter-write-char, r=alexcrichton
This is the logical next step after #24661, but I’m less sure about this one. r? @alexcrichton

Loading status checks…

db0c1cb

bors merged commit 63da18b into rust-lang:master Jun 10, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

SimonSapin deleted the SimonSapin:formatter-write-char branch Jun 10, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 1, 2015

std: Fix formatting flags for chars
This recently regressed in rust-lang#24689, and this updates the `Display` implementation to take formatting flags into account. Closes rust-lang#26625

98566ea

alexcrichton referenced this pull request Jul 1, 2015
Merged
std: Fix formatting flags for chars #26698

bors added a commit that referenced this pull request Jul 1, 2015

Auto merge of #26698 - alexcrichton:char-fmt, r=huonw
This recently regressed in #24689, and this updates the `Display` implementation to take formatting flags into account. Closes #26625

Loading status checks…

1fc0f68

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 8, 2015

std: Fix formatting flags for chars
This recently regressed in rust-lang#24689, and this updates the `Display` implementation to take formatting flags into account. Closes rust-lang#26625

8dff253

jroesch added a commit to jroesch/rust that referenced this pull request Jul 21, 2015

std: Fix formatting flags for chars
This recently regressed in rust-lang#24689, and this updates the `Display` implementation to take formatting flags into account. Closes rust-lang#26625

be62d7e

thepowersgang added a commit to thepowersgang/rust that referenced this pull request Jul 25, 2015

std: Fix formatting flags for chars
This recently regressed in rust-lang#24689, and this updates the `Display` implementation to take formatting flags into account. Closes rust-lang#26625

02b0a08

rust-lang/rust

Join GitHub today

Add a write_char method to std::fmt::Formatter. #24689

Conversation

SimonSapin commented Apr 22, 2015

rust-highfive assigned alexcrichton Apr 22, 2015

This comment has been minimized.

rust-highfive commented Apr 22, 2015

This comment has been minimized.

alexcrichton commented Apr 28, 2015

This comment has been minimized.

SimonSapin commented Apr 28, 2015

This comment has been minimized.

alexcrichton commented Apr 28, 2015

alexcrichton added T-libs I-needs-decision labels May 26, 2015

This comment has been minimized.

alexcrichton commented Jun 10, 2015

alexcrichton removed the I-needs-decision label Jun 10, 2015

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from b3c46d0 to 10fbce3 Jun 10, 2015

This comment has been minimized.

SimonSapin commented Jun 10, 2015

alexcrichton reviewed Jun 10, 2015 View changes

This comment has been minimized.

alexcrichton Jun 10, 2015

This comment has been minimized.

alexcrichton commented Jun 10, 2015

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from 10fbce3 to 63da18b Jun 10, 2015

This comment has been minimized.

alexcrichton commented Jun 10, 2015

bors added a commit that referenced this pull request Jun 10, 2015

This comment has been minimized.

bors commented Jun 10, 2015

This comment has been minimized.

bors commented Jun 10, 2015

Hide details View details bors merged commit 63da18b into rust-lang:master Jun 10, 2015 2 checks passed

2 checks passed

SimonSapin deleted the SimonSapin:formatter-write-char branch Jun 10, 2015

SimonSapin reviewed Jun 29, 2015 View changes

This comment has been minimized.

SimonSapin Jun 29, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 1, 2015

alexcrichton referenced this pull request Jul 1, 2015

std: Fix formatting flags for chars #26698

bors added a commit that referenced this pull request Jul 1, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 8, 2015

jroesch added a commit to jroesch/rust that referenced this pull request Jul 21, 2015

thepowersgang added a commit to thepowersgang/rust that referenced this pull request Jul 25, 2015

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from `b3c46d0` to `10fbce3` Jun 10, 2015

alexcrichton reviewed Jun 10, 2015

View changes

SimonSapin force-pushed the SimonSapin:formatter-write-char branch from `10fbce3` to `63da18b` Jun 10, 2015

bors merged commit `63da18b` into rust-lang:master Jun 10, 2015
2 checks passed

SimonSapin reviewed Jun 29, 2015

View changes