Skip to content

Implement Regex.mapOutput #455

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

Closed
wants to merge 1 commit into from
Closed

Implement Regex.mapOutput #455

wants to merge 1 commit into from

Conversation

Azoy
Copy link
Contributor

@Azoy Azoy commented May 28, 2022

@Azoy Azoy requested review from milseman, rxwei and natecook1000 May 28, 2022 19:49
@Azoy
Copy link
Contributor Author

Azoy commented May 28, 2022

@swift-ci please test

let string1 = "Hello"

let regex4 = regex3.mapOutput {
"\($0) world!"[...]
Copy link
Member

Choose a reason for hiding this comment

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

Curious here — does this cause a problem if you leave off the [...]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously if the output type was just Substring it would've returned only the whole match instead of actually transforming the output, so I'm testing that even with just a Substring output type we still attempt to transform.

Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a test of the behavior when dropping an output-mapped regex in a builder block?

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.

This is a short-term band aid (which might be necessary). What we need for mapOutput is a DSLTree node that will transform / map the actual outputs. E.g. contains(captureNamed:) would honor any tuple labels that were added or removed.

We can merge something like this (but with more tests) as a stop-gap, open an issue, and publish a release note about this being a known problem. Should we also fatal-error if those APIs are called or if the mapping is nested in a way we don't fully support?

@@ -23,6 +23,8 @@ extension Regex {
public let range: Range<String.Index>

let value: Any?

var transform: ((Any) -> Output)?
Copy link
Member

Choose a reason for hiding this comment

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

What's the tradeoff of storing transform here vs just doing it when making a Match object?

@milseman
Copy link
Member

I.e. mapOutput should close the semantic gap between the DSL and the literals, enabling these tests: https://github.com/apple/swift-experimental-string-processing/blob/main/Tests/RegexBuilderTests/AnyRegexOutputTests.swift#L252

@Azoy Azoy closed this Jan 9, 2023
@Azoy Azoy deleted the mapOutput branch January 9, 2023 19:18
@wwalexander
Copy link

Hey, I've been lurking this issue for a little while. @Azoy, do you have any info on the status of this feature?

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.

4 participants