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

Alternative literal path segments for route definitions (#2815) #2920

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

987Nabil
Copy link
Contributor

fixes #2815
/claim #2815

@@ -458,6 +530,7 @@ object PathCodec {
private[http] sealed trait Opt
private[http] object Opt {
final case class Match(value: String) extends Opt
final case class MatchAny(values: Set[String]) extends Opt
Copy link
Member

Choose a reason for hiding this comment

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

Good to use a set here. We can do even better than that by producing a finite state machine that produces true for a match, and false otherwise (due to the char-by-char nature of testing for string equality). But, separate issue and PR. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean some structure, where we have like n states, where n is max length of the alternatives and there is a way to progress to the next state with the right char, true if the string ends or false for any other char, right?

Copy link
Member

@jdegoes jdegoes left a comment

Choose a reason for hiding this comment

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

Looks great, just one relatively minor change!

@987Nabil 987Nabil force-pushed the alternative-path-segments branch 4 times, most recently from f9fc488 to f7c392e Compare June 19, 2024 21:21
@987Nabil 987Nabil force-pushed the alternative-path-segments branch from f7c392e to a44bd4d Compare June 20, 2024 18:13
@jdegoes jdegoes merged commit 5d57d73 into zio:main Jun 23, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple Strings to a single Handler
2 participants