Skip to content

Add conversion to/from gleam types for CBOR tags 1-4#8

Open
karlsson wants to merge 1 commit intoBeaudidly:mainfrom
karlsson:minor-tags
Open

Add conversion to/from gleam types for CBOR tags 1-4#8
karlsson wants to merge 1 commit intoBeaudidly:mainfrom
karlsson:minor-tags

Conversation

@karlsson
Copy link

This PR will decode time tags (1,2) and bignum (3,4) to gleam Timestamp (gleam_time type) and Int respectively. It will also encode Timestamps to tag 0 (only) and bignum integers to tag 3 or 4.
This is a proposal that may need some more thought and should maybe not be included the 1.0 release?

@Beaudidly
Copy link
Owner

@karlsson Something I was thinking about is if we want to configure the decode process, and potentially the encoding part.

For decoding, that could look like:

  • Option to decode to just the value instead of CBTagged(<tag>, <value>) when decoding tags.
    • Rationale here could be that a user doesn't want the check and just accept the value. Also could be useful for the JS target as we rely on Erlang here.
  • Option to decode tags as Timestamp (here)
    • Allows users to avoid needing the gleam_timestamp package as a dependency 🤔

Encoding:

  • Size of floats
    • Right now, we always encode floats as 8 bytes.
    • This could just be done by changing Float to contain the byte-size as well?

@karlsson
Copy link
Author

Yes, and concerning tags 2 and 3 (Bignum) they are automatically handled by Erlang integers, but for javascript I think you need BigInt which is handled by the gleam library bigi, but I do not think there are many use cases for Bignums in applications which use the CBOR protocol.
So instead, maybe just keep as it is, and then it is up to the user to handle any tags and convert to "native" types. Could be complemented with support libraries to handle timestamps, bignum, variable size floats etc.?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants