Skip to content
This repository was archived by the owner on Aug 15, 2021. It is now read-only.

Add tags support behind feature flag#151

Closed
vmx wants to merge 1 commit intopyfisch:masterfrom
vmx:tagstruct
Closed

Add tags support behind feature flag#151
vmx wants to merge 1 commit intopyfisch:masterfrom
vmx:tagstruct

Conversation

@vmx
Copy link

@vmx vmx commented Sep 25, 2019

Tags are stored as Newtype Structs with the special name _TagStruct. The tags
are represented as a struct tuple consisting of the tag identifier and a value.

Example:

let tag = Value::Tag(123, Box::new(Value::Text("some value".to_string())));
let tag_encoded = to_vec(&tag).unwrap();

This implementation is heavily based on the msgpack-rust library, when support
for extensions were added 1. They have the same problem as CBOR has, that
such an extension/tagging type is not directly supported by Serde, hence a
workaround is needed. For me it makes sense to use a similarly working workaround
across different libraries.

This commit also contains an example on how to use custom types as tags. It can
be run via:

cargo run --example tags --features tags

This is an alternate proposal to #129.

Tags are stored as Newtype Structs with the special name `_TagStruct`. The tags
are represented as a struct tuple consisting of the tag identifier and a value.

Example:

    let tag = Value::Tag(123, Box::new(Value::Text("some value".to_string())));
    let tag_encoded = to_vec(&tag).unwrap();

This implementation is heavily based on the msgpack-rust library, when support
for extensions were added [1][2]. They have the same problem as CBOR has, that
such an extension/tagging type is not directly supported by Serde, hence a
workaround is needed. For me it makes sense to use a similarly working workaround
across different libraries.

This commit also contains an example on how to use custom types as tags. It can
be run via:

    cargo run --example tags --features tags

[1]: 3Hren/msgpack-rust@a34ab8f
[2]: 3Hren/msgpack-rust#216
@vmx
Copy link
Author

vmx commented Sep 27, 2019

I really thought this is a good idea. But after digging into all the discussions about tag support in Serde, I also come to the conclusion that this should be properly solved within Serde so that there's interoperability between different formats.

I would want to revive the approach take at serde-rs/serde#408. @pyfisch do you still have your tagged_values branch of cbor lying around somewhere (no worries if not, it shouldn't be too hard to implement)?

@pyfisch
Copy link
Owner

pyfisch commented Oct 5, 2019

Hi @vmx, integrating with serde is a great idea. The tagged_values branch is still available on GitHub isn't it? https://github.com/pyfisch/serde/tree/tagged_values

I'd probably do some things differently than in this old branch. For example I'd use a TaggedVisitor as suggested in the discussion.

@pyfisch
Copy link
Owner

pyfisch commented Oct 5, 2019

I will send a PR for the serialization part to the serde repo soon.

@pyfisch pyfisch mentioned this pull request Oct 6, 2019
15 tasks
@vmx
Copy link
Author

vmx commented Oct 6, 2019

@pyfisch Wow, that's great to hear. I so far have ported the old version to master and I started playing around with it. I thought about creating a small example with a tag that is used in both, CBOR and msgpack, which is then (correctly) serialized/deserialized.

So please let me know if the new version of the patch for Serde is up. I really want to see this happen and I also have time to spend on this if there's a chance to get this upstream :)

@pyfisch
Copy link
Owner

pyfisch commented Oct 7, 2019

See also #157

@pyfisch
Copy link
Owner

pyfisch commented Oct 7, 2019

@vmx would you mind sharing the branch ported to master? I've just created a PR for serde to include tag serialization. (serde-rs/serde#1643) Having the example you suggest would be great to demonstrate why the feature is useful.

@vmx
Copy link
Author

vmx commented Oct 7, 2019

@pyfisch I haven't done that much, I pushed whatever I currently have though:

I kind of stopped when I saw that you're working on it again. But I'm happy to help, when you tell me what I should work on (my Serde knowledge is still pretty basic, though).

@rklaehn
Copy link
Contributor

rklaehn commented Jan 11, 2020

Tags are on master, so I guess we can close this.

@pyfisch pyfisch closed this Jan 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants