chanced / jsonptr

JSON Pointer (RFC 6901) implementation for Rust
Apache License 2.0
44 stars 4 forks source link

Should `Token` serialize as decoded? #36

Closed asmello closed 2 months ago

asmello commented 2 months ago

Currently Token implements serde::ser::Serialize by serializing its decoded string representation. I'm not sure this is desirable, as it's inconsistent to how the underlying Pointer would be serialized. It may also require allocation.

Additionally, the same applies for serde::de::Deserialize. If we serialize as decoded, we must explicitly encode on deserialization.

chanced commented 2 months ago

The thinking behind this was that if you are working with a token in isolation, you're likely looking for the decoded value. Whether that's using it as a key or what have you. The behavior is consistent across as_str, ToString, and such.

When dealing with the entire pointer though, you probably want the fully-encoded string. The pointer itself would be mostly meaningless, I think, if it were decoded. You'd have no way to distinguish '/' in a field name from a seperator.

chanced commented 2 months ago

Also, being able to make the assumption that a token coming from a string needs to be encoded enables infallible conversion from &str to Token.

asmello commented 2 months ago

Yeah, to be clear I really wasn't sure what was the best behaviour here. I don't like that whatever choice made ends up being implicit. We should make sure to document it well.

That said, I think I agree with you that the decoded value makes the most sense, so will close out since that's the current behaviour.