-
Notifications
You must be signed in to change notification settings - Fork 50
Update algorithms methods to match proposal #330
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
Conversation
This eliminates PredicateConsumer from the implementation so that the closure can throw.
This plumbs those parameters down into the SplitCollection type, and removes Collection conformance for now because (a) we aren't using it, and (b) it looks tricky to implement properly.
Better test coverage for both the regex and collection variants of these two methods, plus overloads for contains(_:) to win over Foundation's version from the overlay. Also fixes an error in TwoWaySearcher that tried to offset past the searched string's endIndex; might be a sign of something awry, but tests seem to be otherwise succeeding.
@swift-ci Please test |
1c02a78
to
a1ae820
Compare
@swift-ci Please test |
@swift-ci Please test |
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.
Looking great!
public func contains(_ other: Substring) -> Bool { | ||
firstRange(of: other) != nil | ||
} | ||
} |
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.
Do we need to add these to the proposal?
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.
I don't think so - we just need to make sure these are winning the overload resolution vs Foundation's overlay version. However we do that is just an implementation detail.
@escaping
/rethrows
closuressplit
parametersDocumentation commentsComing in a separate PR