Render literals & break up giant renderRead function to allow reuse of e.g. renderExpr across renderings
#317
Conversation
robmwalsh
changed the title
renderRead function to allow reuse of e.g. renderExpr across renderings
|
@robmwalsh this is so much needed 🙏 . Could you resolve the conflicts? Unfortunately there were couple of PRs that also modified the rendering and also split the function for reuse. |
…iterals � Conflicts: � core/jvm/src/main/scala/zio/sql/expr.scala � postgres/src/main/scala/zio/sql/postgresql/PostgresModule.scala � postgres/src/test/scala/zio/sql/postgresql/FunctionDefSpec.scala
|
really wish I did this one before the hackathon! It was interesting to see all the hacks people added to get their tests working... I broke a couple of tests which are now btw I've noticed that |
|
No, it's not fmt, it's me :). Sometimes when I resolve conflicts when merging master to someone else's branch, I make mistakes and then the check fails. But I merge anyway, because it would take forever to contact eveyone. |
| ) | ||
|
|
||
| private def postgresStringEscape(s: String): String = s""" '${s}' """ | ||
| } @@ ignore //todo fix - select(PgClientEncoding())? |
There was a problem hiding this comment.
from smth is required now. We need to rethink Read.
There was a problem hiding this comment.
I raised this #323 for this one. I think we should rename Read.Select to Read.SelectFrom and SelectBuilder to Read.Select, both extending a common trait which the renderer can match on.
I think the underlying issue with this one I think is just that it needs parens and other Function0s don't.
This is one of the changes I was talking about in my comment on #290 (comment)
|
This is a bit different, I merged master, ran This stems from the fact that scalafmt isn't entirely idempotent. There are a few issues floating about but they keep closing them because it's a problem with one of their dependencies (e.g. scalameta/scalafmt#917). Can we please merge this now? The ones that I broke will be resolved with the changes I requested here: #290 (comment) |
Render literals & break up giant `renderRead` function to allow reuse of e.g. `renderExpr` across renderings
not a complete solution but at least fixes string rendering and should make rendering other types easier (at least shows the pattern)
partial solution to #311
also made the rendering functions a bit more user friendly and modular