shamblett / cbor

A CBOR implementation for Dart
MIT License
36 stars 15 forks source link

API rewrite #29

Closed nicbn closed 2 years ago

nicbn commented 2 years ago

See #28

Given the API has to be completely rewritten anyways, I am also restructuring and rewriting the code. Let me know if anything is not of your liking.

Check here for documentation on the decoder.

For dealing with half-precision I used the IEEE 754 library, which will also allow the encoder to encode with 16 bit precision even if the value is not an integer, as long as the conversion is completely lossless.

shamblett commented 2 years ago

OK, I need to have a deeper look but initially could you rename the files to start with cbor_, I know this is not absolutely necessary but the other files already are and it would look odd to mix them in the same package.

I think it would be a good idea at this point to take the cbor_decoder_rfc_test suite and update it to use the new decoder, this will give us confidence this is is all going to be OK before we press on, it will also highlight any omissions, its hard to do this just looking at the code alone, might as well use the tests to help us.

nicbn commented 2 years ago

The semantics of cbor_ seem to indicate that the file is part of cbor, which is a pattern that is discouraged in favor of creating mini libraries that use export and import. The current files will be removed/moved to fit in, so by the end there shouldn't be this naming mismatch.

I think it would be a good idea at this point to take the cbor_decoder_rfc_test suite and update it to use the new decoder

Fair enough, I'm going to update this test suit before proceeding. Thanks for your input.

shamblett commented 2 years ago

No problems with the move to the newer mini library model for the whole package, as you say this is now the preferred way of doing things.

nicbn commented 2 years ago

I have updated cbor_decoder_rfc_tests. The tests also give a good example of usage. All tests are passing, but several bugs were fixed in the way.

shamblett commented 2 years ago

Thanks, seems to be progressing nicely, the decoder RFC tests look much better

nicbn commented 2 years ago

Implementation for the tests and the encoder are done Some of the previous supplementary tests were removed, and new supplementary tests were added

nicbn commented 2 years ago

I have removed the list and map builder examples (as the new API does not supply this anymore), as well as the complex encode example, and instead added the indefinite length to the simple encode.

I think this PR is ready to be merged to the issue18 branch, but some extra things are needed before this is ready to publish:

shamblett commented 2 years ago

Yep, there's a lot of work done here, I've merged this, helps me also I can now run the tests etc.

I've updated #28 with your comments above so we can track further PR's through this issue.

You can forget about coverage, this is something I used to do but am removing it from my other packages. If you feel it would help you with this work then feel free to generate coverage if you wish, if not just leave it.