Add PathVar map, emap #7310
Add PathVar map, emap #7310
Conversation
Not sure what that one does. CI runs |
| @@ -40,6 +40,7 @@ trait RequestDsl extends Methods with Auth { | |||
| val IntVar: impl.IntVar.type | |||
| val LongVar: impl.LongVar.type | |||
| val UUIDVar: impl.UUIDVar.type | |||
| val StringVar: impl.StringVar.type | |||
There was a problem hiding this comment.
This is not backwards-compatible: we cannot add an abstract method to an open trait, since what happens to all the existing implementations that are missing this one?
There was a problem hiding this comment.
Uh that's annoying. I don't know what could be done in this direction that doesn't end up changing RequestDsl.
There was a problem hiding this comment.
So I don't know anything about this corner of http4s and why it's written this way, but this would fix the compat issues:
| val StringVar: impl.StringVar.type | |
| lazy val StringVar: impl.StringVar.type = impl.StringVar |
There was a problem hiding this comment.
I was lazy and didn't check it, I'll try again later and run the right mima check
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
|
MiMa no longer complains, and yet this feels less useful now 🥲. Before making this PR I didn't know you could just make an extractor (my fault, hadn't read the doc) so this seemed like a great idea. I'll leave it up to reviewers to decide if it's worth it. |
| new PathVar[B](str => cast(str).map(f)) | ||
|
|
||
| def emap[B](f: A => Option[B]): PathVar[B] = | ||
| new PathVar[B](str => cast(str).flatMap(a => f(a).toRight(new NoStackTrace {}).toTry)) |
There was a problem hiding this comment.
A little bit of bikeshedding, but this feels like too many allocations for the simple instantiation... Considering the PathVar nature (something that's going to be used for every request on a given path), could we tweak this?
Follow up from #2405
The motivation is that, as a user, in order to use make your own
PathVaryou have to make your own full extractor (e.g.LocalDateVarin https://http4s.org/v0.23/docs/dsl.html#handling-path-parameters) or make your ownorg.http4s.dsl.implpackage so that you can accessPathVarand its constructor.I implemented
mapandemapinPathVar, and addedStringVarwhich can be used as a starting point for creating customPathVars. @diesalbla suggested a "super-traitPathMatch[A]" but I did not understand what the purpose would be, I'll happily make the adjustment.As it is this would encourage user-defined PathVars to be
vals and notobjects, which becomes inconsistent with QueryParams for example. If this is a problem (I really have no opinion) then something likeabstract class CustomPathVar[A](cast: String => Option[A]) extends PathVar(...)would still be flexible enough, though less compositional.I don't know if any of this is MiMa-approved (I ran mimaFindBinaryIssues, does that do anything?).