Skip to content
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

Add support for .cbor control operator #7

Merged
merged 2 commits into from
Oct 1, 2023

Conversation

KtorZ
Copy link
Contributor

@KtorZ KtorZ commented Sep 28, 2023

This operator is pretty useful for embedding CBOR in CBOR; and relatively low-hanging fruit to implement as we can recursively re-use the existing validation.

  • Implemented validation
  • Fixed CDDL parser
  • Added few unit tests
  • Updated README.md

Note
Since the validation rely on serde_cbor, I've only conditionally included it. If the library is imported without that feature, then the behavior is more or less the current one (i.e. a Validation failure for an unsupported control operator).

  This operator is pretty useful for embedding CBOR in CBOR; and
  relatively low-hanging fruit to implement as we can recursively re-use
  the existing validation.
@ericseppanen
Copy link
Owner

Thanks for this!

Looks like cargo build --no-default-features --features serde_json needs fixing, but other than that everything looks good.

@KtorZ
Copy link
Contributor Author

KtorZ commented Oct 1, 2023

Indeed! I thought that the inline cfg! macro would do the trick but it did not. Falling back to a conditional function overloading work as expected it seems.

  Turns out the inline 'cfg' macro doesn't work as the code is still included even if serde_cbor isn't required. The overloading fixes this.
@ericseppanen ericseppanen merged commit 154798e into ericseppanen:master Oct 1, 2023
5 checks passed
@ericseppanen
Copy link
Owner

I published 1.6.2. Thanks again!

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