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

Add some examples to std::string #30770

Merged
merged 1 commit into from
Jan 14, 2016
Merged

Conversation

steveklabnik
Copy link
Member

Fixes #30345

I'm not sure if there's anything else that belongs here. Thoughts?

@rust-highfive
Copy link
Collaborator

r? @brson

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

@ollie27
Copy link
Member

ollie27 commented Jan 7, 2016

I think in order to actually fix #30345 there should be a mention of the word "concatenation", perhaps in the description and examples for push_str.

//!
//! let bytes = sparkle_heart.into_bytes();
//!
//! assert_eq([240, 159, 146, 150, bytes);
Copy link
Member

Choose a reason for hiding this comment

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

Should probably be:

assert_eq!(bytes, [240, 159, 146, 150]);

Copy link
Member Author

Choose a reason for hiding this comment

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

For what reason?

@steveklabnik
Copy link
Member Author

Good call on putting the word 'concatenation'

//!
//! let bytes = sparkle_heart.into_bytes();
//!
//! assert_eq([240, 159, 146, 150], bytes);
Copy link
Contributor

Choose a reason for hiding this comment

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

!

Copy link
Member

Choose a reason for hiding this comment

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

and the arguments still need to be swapped because PartialEq<Vec<u8>> is not implemented for [u8; 4]

Copy link
Member Author

Choose a reason for hiding this comment

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

ugh, how did this pass for me locally, then?

Copy link
Contributor

Choose a reason for hiding this comment

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

What's your test setup / make command?
On Jan 8, 2016 11:02, "Steve Klabnik" notifications@github.com wrote:

In src/libcollections/string.rs
#30770 (comment):

+//! +//! +//! If you have a vector of valid UTF-8 bytes, you can make aString out of +//! it. You can do the reverse too. +//! +//!rust
+//! let sparkle_heart = vec![240, 159, 146, 150];
+//!
+//! // We know these bytes are valid, so we'll use unwrap().
+//! let sparkle_heart = String::from_utf8(sparkle_heart).unwrap();
+//!
+//! assert_eq!("💖", sparkle_heart);
+//!
+//! let bytes = sparkle_heart.into_bytes();
+//!
+//! assert_eq([240, 159, 146, 150], bytes);

ugh, how did this pass for me locally, then?


Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/30770/files#r49203568.

@brson
Copy link
Contributor

brson commented Jan 12, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Jan 12, 2016

📌 Commit 51b11ca has been approved by brson

@bors
Copy link
Contributor

bors commented Jan 12, 2016

⌛ Testing commit 51b11ca with merge 18f97a7...

@bors
Copy link
Contributor

bors commented Jan 12, 2016

💔 Test failed - auto-mac-64-nopt-t

@steveklabnik
Copy link
Member Author

@bors: r=brson rollup

:(

@bors
Copy link
Contributor

bors commented Jan 12, 2016

📌 Commit b6cc099 has been approved by brson

Manishearth added a commit to Manishearth/rust that referenced this pull request Jan 14, 2016
Fixes rust-lang#30345

I'm not sure if there's anything else that belongs here. Thoughts?
bors added a commit that referenced this pull request Jan 14, 2016
@bors bors merged commit b6cc099 into rust-lang:master Jan 14, 2016
@steveklabnik steveklabnik deleted the gh30345 branch June 19, 2016 20:30
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