-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Rename SliceConcatExt::connect to join #26957
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1025,6 +1025,17 @@ pub trait SliceConcatExt<T: ?Sized> { | |
#[stable(feature = "rust1", since = "1.0.0")] | ||
fn concat(&self) -> Self::Output; | ||
|
||
/// Flattens a slice of `T` into a single value `Self::Output`, placing a | ||
/// given separator between each. | ||
/// | ||
/// # Examples | ||
/// | ||
/// ``` | ||
/// assert_eq!(["hello", "world"].join(" "), "hello world"); | ||
/// ``` | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
fn join(&self, sep: &T) -> Self::Output; | ||
|
||
/// Flattens a slice of `T` into a single value `Self::Output`, placing a | ||
/// given separator between each. | ||
/// | ||
|
@@ -1034,6 +1045,7 @@ pub trait SliceConcatExt<T: ?Sized> { | |
/// assert_eq!(["hello", "world"].connect(" "), "hello world"); | ||
/// ``` | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
#[deprecated(since = "1.3.0", reason = "renamed to join")] | ||
fn connect(&self, sep: &T) -> Self::Output; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably best to just have this default to calling join so you don't need multiple impls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it backwards-incompatible to do anything other than having There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, he even had to fix libcollections/str.rs, so it's clearly not backward compatible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The trait is marked unstable but the individual functions are marked as stable. What does that mean in terms of backward-compatibility? I tried to make my changes backwards compatible for users of the trait but I hadn't considered implementors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eddyb All implementors must have given an impl of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without a default on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to me that providing a default on one of the functions is a good idea. I'm just not sure which one. If we:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yes. Sorry I saw that this PR was breaking all implementations anyway, Longterm I think connect-defaults-to-join is the right one. Regardless, On Sat, Jul 11, 2015 at 10:45 AM, Eduard Burtescu [email protected]
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I'll change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having a seperate "rename" and "fallout" commit seems reasonable. On Sat, Jul 11, 2015 at 1:42 PM, Wesley Wiser [email protected]
|
||
} | ||
|
||
|
@@ -1049,7 +1061,7 @@ impl<T: Clone, V: Borrow<[T]>> SliceConcatExt<T> for [V] { | |
result | ||
} | ||
|
||
fn connect(&self, sep: &T) -> Vec<T> { | ||
fn join(&self, sep: &T) -> Vec<T> { | ||
let size = self.iter().fold(0, |acc, v| acc + v.borrow().len()); | ||
let mut result = Vec::with_capacity(size + self.len()); | ||
let mut first = true; | ||
|
@@ -1059,6 +1071,10 @@ impl<T: Clone, V: Borrow<[T]>> SliceConcatExt<T> for [V] { | |
} | ||
result | ||
} | ||
|
||
fn connect(&self, sep: &T) -> Vec<T> { | ||
self.join(sep) | ||
} | ||
} | ||
|
||
/// An iterator that yields the element swaps needed to produce | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since 1.3.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gankro Thanks! Fixed in 5ee801a