Skip to content

Pitch: String processing algorithms #188

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

Merged
merged 15 commits into from
Mar 22, 2022

Conversation

itingliu
Copy link
Contributor

No description provided.

Tim Vermeulen and others added 4 commits February 24, 2022 00:52
    - Motivation for adding algorithms
    - Motivation for `CustomRegexComponent`
    - Design for added algorithms
    - Design for `CustomRegexComponent`
- Add a few doc comments
Update pitch: 

Restructure the pitch to this structure:

- Motivation for adding algorithms
- Motivation for CustomRegexComponent
- Design for added algorithms
- Design for CustomRegexComponent

Add a few doc comments. 
Update Foundation example.
@itingliu itingliu changed the title Pr/string processing pitch update Pitch: String processing algorithms Feb 25, 2022
@itingliu itingliu marked this pull request as draft February 25, 2022 19:02
#### Trim prefix

```swift
extension Collection {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find myself squinting to see the differences between these functions. I wonder if it's easier to review if we group them by extension other than algorithms, like

extension Collection where Element: Equatable {
    public func contains<S: Sequence>(_ other: S) -> Bool
    public func trimmingPrefix<Prefix: Collection>(_ prefix: Prefix) -> SubSequence
}

extension BidirectionalCollection where SubSequence == Substring {
    public func contains<R: RegexProtocol>(_ regex: R) -> Bool
    public func starts<R: RegexProtocol>(with regex: R) -> Bool
}

Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

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

Just some wordsmithing and editorial adjustments. This is looking good so far, though there's opportunity to have less redundancy.


You can conform your custom parser to `CustomRegexComponent`. Conformance is simple: implement the `match` function to return the upper bound of the matched substring, and the type represented by the matched range. It inherits from `RegexProtocol`, so you will be able to use it with all of the string algorithms that take a `RegexProtocol` type.

Here, we use Foundation framework's `FloatingPointFormatStyle<Double>.Currency` as an example. `FloatingPointFormatStyle<Double>.Currency` would conform to `CustomRegexComponent` by implementing the `match` function, with `Match` being a `Double`. It could also add a static function `.localizedCurrency(code:)` as a member of `RegexProtocol`, so you can refer to it as `.localizedCurrency(code:)` in the `Regex` result builder.
Copy link
Member

Choose a reason for hiding this comment

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

It really seems like this should be merged into the "proposed solution" sub-section, where the example's fresh in the reader's mind.

itingliu and others added 3 commits February 26, 2022 08:03
Editorial changes

Co-authored-by: Michael Ilseman <michael.ilseman@gmail.com>
- Add a comparison table for Python string functions and those of Swift
- Add header docs for all except for `replacing`
@milseman
Copy link
Member

@itingliu let me know when's a good time to do another pass over the prose. I don't want to conflict or interrupt any reorganizing you're doing.

@itingliu itingliu marked this pull request as ready for review February 28, 2022 23:35
itingliu and others added 2 commits March 11, 2022 14:05
Co-authored-by: Michael Ilseman <michael.ilseman@gmail.com>
Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I think some of the "backwards" future direction is salvageable and should be mentioned, but it doesn't need to be presented as a big deal.

After that, ship it!

@itingliu
Copy link
Contributor Author

Overall LGTM. I think some of the "backwards" future direction is salvageable and should be mentioned, but it doesn't need to be presented as a big deal.

After that, ship it!

I reworked it a little bit and added it back. Thanks!

@milseman
Copy link
Member

@swift-ci please test

@itingliu itingliu merged commit e087320 into swiftlang:main Mar 22, 2022
@itingliu itingliu deleted the pr/string-processing-pitch-update branch March 22, 2022 21:33
rxwei pushed a commit to rxwei/swift-experimental-string-processing that referenced this pull request Mar 28, 2022
* Add string processing algorithms pitch

Co-authored-by: Tim Vermeulen <tvermeulen@apple.com>
Co-authored-by: Michael Ilseman <michael.ilseman@gmail.com>
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.

3 participants