-
Notifications
You must be signed in to change notification settings - Fork 188
Move off of RegexBuilder for URL.Template #1273
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci please test |
} | ||
} | ||
} | ||
self.operatorRegex = try! Regex(#"([\+#.\/;\?&])?"#) |
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 you know if it makes a difference whether you create a Regex this way or the other one that specifies the output type?
init(
_ pattern: String,
as outputType: Output.Type = Output.self
) throws
For example would it help the performance since we can bypass parsing the result from AnyOutputRegex
?
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 have no idea if that makes any difference. Happy to change it, if that helps. These are only every created once, though.
@danieleggert Can you rebase to resolve the merge conflict? |
@@ -176,6 +177,7 @@ extension URL.Template: CustomStringConvertible { | |||
} | |||
} | |||
|
|||
@available(FoundationPreview 6.2, *) |
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 this annotation is needed since URL.Template.Element
is an internal type, but otherwise LGTM!
This fixes #1269
This uses
try! Regex("…")
instead ofRegexBuilder
to create regular expressions used by the URL Template implementation.