-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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 remove(where:) to RangeReplaceableCollection #11576
Conversation
@swift-ci please test |
Build failed |
@swift-ci please test linux platform |
@@ -509,6 +509,19 @@ public protocol RangeReplaceableCollection | |||
/// | |||
/// - Complexity: O(*n*), where *n* is the length of the collection. | |||
mutating func removeAll(keepingCapacity keepCapacity: Bool /*= false*/) | |||
|
|||
/// Removes all elements from the collection. |
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.
This summary seems to be wrong..
Given that the names in the proposal have been changed, and the intent behind having an implementation is that people can get actual experience with the proposal, I think this PR should be updated before we run the review.
/// Calling this method may invalidate any existing indices for use with this | ||
/// collection. | ||
/// | ||
/// - Parameter keepCapacity: Pass `true` to request that the collection |
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.
This parameter doesn't seem to exist
@@ -1226,7 +1239,7 @@ extension RangeReplaceableCollection { | |||
/// | |||
/// - Parameter isIncluded: A closure that takes an element of the | |||
/// sequence as its argument and returns a Boolean value indicating | |||
/// whether the element should be included in the returned array. | |||
/// whether the element should be included in the returned collection. |
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.
You might want to make that fix separately from this PR
extension RangeReplaceableCollection where Self: MutableCollection { | ||
/// Removes from the collection all elements that satisfy the given predicate. | ||
/// | ||
/// - Parameter where: A closure that takes an element of the |
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.
The parameter name is predicate
, not where
, thank goodness!
test/stdlib/RemoveElements.swift
Outdated
@@ -0,0 +1,101 @@ | |||
//===--- Filter.swift - tests for lazy filtering --------------------------===// |
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.
Copy/paste error?
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.
derp... thx
f3e48e4
to
8dfa517
Compare
@swift-ci please test |
I am not sure if it matters, but in |
@martinr448 yeah, it also shouldn't use |
8dfa517
to
ba11a23
Compare
@swift-ci please test and merge |
Initial implementation for swiftlang/swift-evolution#675