-
Notifications
You must be signed in to change notification settings - Fork 132
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
fix: Remove extra apply in generated multiple query parameters #287
Conversation
I'm interested to see whether this compiles; this was added to fix #249, it may trip the regression test. I think #250 may have resolved #210 already, but I'm interested as to why you're getting compilation failures about |
Here is an example that breaks, anything else I can help with? Edit: Not sure if related but I am using akka-http
|
Ah, it seems as though this is a divergence of behaviour between 2.11 and 2.12. Finding a structure that compiles in both will be interesting. Are you available to try and fix this? If not, I can take a look after I'm done on my current PR, attempting to address #184 |
I would gladly do it, but I am not even understanding the exact issue here. |
My understanding is as follows: parameter(Symbol("foo").as[String].*).map(Option.apply _) compiled in 2.11 and 2.12. This had the unfortunate effect of returning a parameter(Symbol("foo").as[String].*).map(xs => Option(xs).filterNot(_.isEmpty)) compiles in 2.12 but not 2.11, due to some syntactic ambiguity and related type divergence. parameter(Symbol("foo").as[String].*).map(xs => Option(xs).filterNot(_.isEmpty)).apply compiles in 2.11 and 2.12, and tests pass. That being said, your proffered specification does not compile, exposing a previously untested limitation of this suffixed Can you post the surrounding code that resulted in
please? It sounds like we're ending up with something like: path("foo") {
parameter(Symbol("foo").as[String].*).map(xs => Option(xs).filterNot(_.isEmpty)).apply
} causing that |
I just generated with Where Anyway here is the content of method around the code:
And the full full Routes.scala:
|
Oh. Of course that won't work. |
If you |
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.
Sorry for the delay here, merging now. This'll be in the next release.
Ok -- when rebasing, the tests continued to fail with the same problem that was originally happening. I was able to work around it by putting the |
@blast-hardcheese nice. I tried a couple times, but I was not able to get anything close to the real issue. |
Trying to fix codegen in #210
When generating the akka code it does not compile. After removing it and compiling just the akka part it compiles in my tests. But seems like the tests in this repo fail. Any ideas?
Contributing to Twilio