Skip to content
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

Correctly handle path params with a hyphen in the name #602

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

czechboy0
Copy link
Contributor

Motivation

Fixes #601, check out the issue for details.

Modifications

Adds hyphen to the regular expression that parses out params from the URL template.

Result

Correctly handle path params with hyphens in the name.

Test Plan

Added a unit test.

@czechboy0 czechboy0 marked this pull request as draft July 23, 2024 09:40
@czechboy0
Copy link
Contributor Author

Actually, not just hyphen, but a period is also commonly used in path param names. Let me add that too.

@czechboy0 czechboy0 marked this pull request as ready for review July 23, 2024 09:50
@czechboy0 czechboy0 requested a review from simonjbeaumont July 23, 2024 09:50
Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

Nice patch, thanks for including a test.

@simonjbeaumont simonjbeaumont enabled auto-merge (squash) July 23, 2024 09:55
@simonjbeaumont simonjbeaumont disabled auto-merge July 23, 2024 09:55
@czechboy0 czechboy0 enabled auto-merge (squash) July 23, 2024 09:55
@czechboy0
Copy link
Contributor Author

@simonjbeaumont Thanks for the review. I did try to look for an authoritative source for "allowed characters in path parameter names", but couldn't find anything conclusive. So for now, this is what we support.

@czechboy0 czechboy0 merged commit 9985908 into apple:main Jul 23, 2024
9 checks passed
@czechboy0 czechboy0 deleted the hd-fix-hyphen-path-params branch July 23, 2024 10:17
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hyphen in path variable name results in static path variable name in url
2 participants