Conversation
src/pog.gleam
Outdated
| coerce_value(#(time.hours, time.minutes, seconds)) | ||
| } | ||
|
|
||
| pub fn range(converter: fn(a) -> Value, range: Range(a)) { |
There was a problem hiding this comment.
I aligned the argument order with pog.array, but please check if that’s correct.
There was a problem hiding this comment.
Add the missing type annotation please 🙏
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you! Could you update the changelog also please
src/pog.gleam
Outdated
| coerce_value(#(time.hours, time.minutes, seconds)) | ||
| } | ||
|
|
||
| pub fn range(converter: fn(a) -> Value, range: Range(a)) { |
There was a problem hiding this comment.
Add the missing type annotation please 🙏
| pub type Inclusivity { | ||
| Inclusive | ||
| Exclusive | ||
| } |
There was a problem hiding this comment.
Document the new data types please 🙏
There was a problem hiding this comment.
Done. Please double check if it's correct and clear.
src/pog.gleam
Outdated
| use decoded_atom <- decode.then(atom.decoder()) | ||
| case decoded_atom == atom.create("empty") { | ||
| True -> decode.success(Empty) | ||
| False -> decode.failure(Empty, "empty") |
There was a problem hiding this comment.
What's empty? Shouldn't this be Range?
There was a problem hiding this comment.
As far as I understand, this is a sub decoder designed to decode only a single empty Erlang atom. Its error is always suppressed by the usage of decode.one_of.
I've changed the expected error message to be clearer:
False -> decode.failure(Empty, "`empty` atom")I've also changed the main/parent decoder to collapse errors into something more readable:
decode.one_of(range_decoder, [empty_decoder])
|> decode.collapse_errors("`#(#(t, t), #(bool, bool))` tuple or `empty` atom")Is it better now or could you please explain what I'm missing here?
| ) | ||
| |> disconnect | ||
| } | ||
|
|
There was a problem hiding this comment.
Could you add tests for each of the possible ranges that could be decoded please 🙏 Using postgresql literals sounds good.
src/pog.gleam
Outdated
| use decoded_atom <- decode.then(atom.decoder()) | ||
| case decoded_atom == atom.create("unbound") { | ||
| True -> decode.success(Unbound) | ||
| False -> decode.failure(Unbound, "unbound") |
Add tests
| // this test doesn't work! | ||
| |> assert_roundtrip( | ||
| pog.Range(pog.Bound(1.23, pog.Exclusive), pog.Unbound), | ||
| "numrange", | ||
| encoder, | ||
| decoder, | ||
| ) | ||
| // this test doesn't work! | ||
| |> assert_roundtrip( | ||
| pog.Range(pog.Bound(-3.14, pog.Exclusive), pog.Bound(3.14, pog.Inclusive)), | ||
| "numrange", | ||
| encoder, | ||
| decoder, | ||
| ) |
There was a problem hiding this comment.
These tests are failing with the following errors:
An unexpected error occurred:
Badmatch(<<0, 0, 0, 12, 0, 2, 0, 0, 0, 0, 0, 2, 0, 1, 8, 252>>)
An unexpected error occurred:
Badmatch("\u{0000}\u{0000}\u{0000}\f\u{0000}\u{0002}\u{0000}\u{0000}@\u{0000}\u{0000}\u{0002}\u{0000}\u{0003}\u{0005}x\u{0000}\u{0000}\u{0000}\f\u{0000}\u{0002}\u{0000}\u{0000}\u{0000}\u{0000}\u{0000}\u{0002}\u{0000}\u{0003}\u{0005}x")
This appears to be a bug, but I'm not sure where the root cause is.
There was a problem hiding this comment.
The issue is on the pg_types side. I've created tsloughter/pg_types#21 to track its progress.
2 participants
This PR introduces a generic
Range(t)type to support Postgres range types.Please let me know if there's anything I can improve.