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

String conversion updates #18976

Merged
merged 6 commits into from
Nov 16, 2014
Merged

String conversion updates #18976

merged 6 commits into from
Nov 16, 2014

Conversation

brendanzab
Copy link
Member

Working towards the completion of rust-lang/rfcs#369 (tracked in #18640).

[breaking-change]

  • std::from_str::{FromStr, from_str} have been moved to {core, std}::str.
  • std::to_string::IntoStr has been renamed to IntoString and moved to collections::string.
  • std::to_string::ToString has been moved to collections::string.
  • std::{from_str, to_str} have been removed.

@brendanzab
Copy link
Member Author

cc. @aturon @alexcrichton

@brendanzab brendanzab changed the title Move FromStr to core::str String conversion updates Nov 15, 2014
@brendanzab
Copy link
Member Author

I have not been able to move ToString to collections::string as per the RFC, because it requires format!, and ultimately std::io::MemWriter.

@huonw
Copy link
Member

huonw commented Nov 15, 2014

#18885 should remove the necessity for MemWriter.

@brendanzab
Copy link
Member Author

Oh nice. It would need to be somewhere up the dependence tree from collections though for it to be useful...

@huonw
Copy link
Member

huonw commented Nov 15, 2014

Ah, yes, I suppose the Writer trait is defined in std... ignore me.

@alexcrichton
Copy link
Member

You should be able to implement FormatWriter at least and call the libcore function directly to move ToString into libcollections. Sorry it's a bit painful for now, but it should be able to remove the std::to_string module at least!

@alexcrichton
Copy link
Member

Otherwise this looks great by the way, thanks @bjz!

@brendanzab
Copy link
Member Author

Do you mind if I implement FormatWriter on Vec<u8> and String, and move std::fmt::format to String::format?

@alexcrichton
Copy link
Member

@bjz I'd prefer to not expand the API of String for now if possible.

@brendanzab
Copy link
Member Author

Ok

@brendanzab
Copy link
Member Author

I'm still running the tests locally, but you can check out my latest commit.

@brendanzab
Copy link
Member Author

Ok, it passes my tests

@@ -41,6 +41,7 @@ pub use ops::{Fn, FnMut, FnOnce};
// Reexported functions
pub use iter::{range, repeat};
pub use mem::drop;
pub use str::from_str;
Copy link
Member

Choose a reason for hiding this comment

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

@alexcrichton What do you think about this addition to the prelude? We have very few free fns in the prelude, but this seems like a potentially-reasonable candidate.

Copy link
Member

Choose a reason for hiding this comment

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

@alexcrichton Nevermind, I didn't realize this was already in the std prelude.

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry I'm a little late to the part, it appears you've already discovered what I was going to say! I'd personally like to remove this from the prelude, however, but that can come later with prelude stabilization.

@aturon
Copy link
Member

aturon commented Nov 16, 2014

@bjz This is fabulous, really excited to land this cleanup! My only hesitation is about the addition to the prelude; want to get @alexcrichton's opinion on that. Otherwise, r=me.

@brendanzab
Copy link
Member Author

Which addition did I make?

@aturon
Copy link
Member

aturon commented Nov 16, 2014

@bjz Ah -- I meant the addition of from_str (which was added to the core prelude), but I hadn't realized it was already in the std prelude and was previously impossible to have in the core prelude. So nevermind!

milibopp added a commit to milibopp/gfx-rs that referenced this pull request Nov 17, 2014
The exact version is rustc 0.13.0-dev (0b7b4f075 2014-11-16 22:36:50 +0000) and
the relevant upstream issues are rust-lang/rust#18752 and rust-lang/rust#18976.

Fixes gfx-rs#439.
milibopp added a commit to milibopp/gfx-rs that referenced this pull request Nov 17, 2014
The exact version is rustc 0.13.0-dev (0b7b4f075 2014-11-16 22:36:50 +0000) and
the relevant upstream issues are rust-lang/rust#18752 and rust-lang/rust#18976.

Fixes gfx-rs#439.
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