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

SpannedVisitor field order #789

Closed
shouya opened this issue Sep 25, 2024 · 3 comments · Fixed by #792
Closed

SpannedVisitor field order #789

shouya opened this issue Sep 25, 2024 · 3 comments · Fixed by #792

Comments

@shouya
Copy link

shouya commented Sep 25, 2024

I'm attempting to implement span support for serde_yaml in a way compatible to serde_spanned. Specifically, I'd like to handle the magic struct in deserialize_struct from the DeserializerFromEvents deserializer.

DeserializerFromEvents has a pos field that appear relevant, I'm trying detect the span based on this position cursor. However, due to the way DeserializerFromEvents works, it's impossible to extract end of the span the before parsing the value.

I thought about implementing SpannedDeserializer in a way that just wraps around self: &mut DeserializerFromEvents<'de, 'document>, so it can first spit out the starting pos, then the value, then the ending pos.

However, based on how SpannedVisitor is implemented, it expect a strict order of keys to emit from my deserializer, which makes my plan unfeasible.

Based on my understanding of how serde_spanned works, making SpannedVisitor consume the keys in order of start > value > end (or value > start > end) should make it flexible for use cases like mine. Or, for backward compatibility's sake, maybe it's better to allow SpannedVisitor consume the keys in arbitrary order. Or perhaps I'm doing it wrong and there is a better way to do it?

@epage
Copy link
Member

epage commented Sep 25, 2024

When we controlled both sides, the order didn't matter and the current approach is much simpler to implement.

In making this easier for when its easier to generate the values in a specific order, I think its best to generalize it for maximum compatibility.

@epage
Copy link
Member

epage commented Sep 25, 2024

btw would anything from #635 be useful? It was blocked, pending feedback.

@shouya
Copy link
Author

shouya commented Sep 26, 2024

would anything from #635 be useful?

I don't think it's useful for my particular use case with a streaming parser where the span and value are not known upfront.

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 a pull request may close this issue.

2 participants