Skip to content

Add @RegexComponentBuilder overloads for string processing algorithm. #334

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 2 commits into from
Apr 24, 2022

Conversation

itingliu
Copy link
Contributor

These overloads will allow you to write, e.g.

let _ = str.wholeMatch {
    OneOrMore("a")
    "+"
    OneOrMore("a")
}

instead of

let _ = str.wholeMatch(of: Regex {
    OneOrMore("a")
    "+"
    OneOrMore("a")
})

These overloads will allow you to write, e.g.

```swift
let _ = str.wholeMatch {
 	OneOrMore("a")
 	"+"
	OneOrMore("a")
}
```

instead of

```swift
let _ = str.wholeMatch(of: Regex {
	OneOrMore("a")
    "+"
    OneOrMore("a")
})
```
/// - content: A closure to produce a `RegexComponent` to replace.
/// - Returns: A new collection in which all occurrences of subsequence
/// matching `regex` in `subrange` are replaced by `replacement`.
public func replacing<Replacement: 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'm still not quite sure about these replace ones. With the simplest one that has default value for subrange and maxReplacements, you'd do

let _ = str.replacing(with: "xxx") {
    OneOrMore("a")
    "+"
    OneOrMore("a")
}

But this reads backwards to me and doesn't translate great.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. But seems pretty reasonable given the syntactic benefit it offers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I'll leave them here for practicality

@itingliu itingliu requested review from milseman and rxwei April 23, 2022 04:37
public func split(
maxSplits: Int = Int.max,
omittingEmptySubsequences: Bool = true,
@RegexComponentBuilder _ separator: () -> some RegexComponent
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this argument label, so that when the user is not using a trailing closure they will be using a separator: label to clarify the argument?

Suggested change
@RegexComponentBuilder _ separator: () -> some RegexComponent
@RegexComponentBuilder separator: () -> some RegexComponent

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 changed the argument labels of the builder functions to match their non-builder counterparts.

/// - content: A closure to produce a `RegexComponent` to replace.
/// - Returns: A new collection in which all occurrences of subsequence
/// matching `regex` in `subrange` are replaced by `replacement`.
public func replacing<Replacement: Collection>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. But seems pretty reasonable given the syntactic benefit it offers.

/// - Parameter content: A closure to produce a `RegexComponent` to search for.
/// - Returns: A collection of matches of `regex`.
public func matches<R: RegexComponent>(
@RegexComponentBuilder _ content: () -> R
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we keep of:? Even though it disappears as a trailing closure, the user may choose not to use a trailing closure for clarity, e.g. prefix(where: { $0 != 1 })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. I wasn't sure about this. Thanks

/// Match part of the regex, starting at the beginning.
/// - Parameter content: A closure to produce a `RegexComponent` to match against.
/// - Returns: The match if there is one, or `nil` if none.
@available(SwiftStdlib 5.7, *)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for availability in the proposal.

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

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

These additions LGTM! I'll make some documentation revisions once they're in for consistency with my other changes.

@itingliu
Copy link
Contributor Author

@swift-ci please test

@itingliu itingliu merged commit 7b737e3 into swiftlang:main Apr 24, 2022
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