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

Improve some crate::de::Deserialize impls by privately introducing CowBytes<'a> #24

Closed
wants to merge 2 commits into from

Conversation

timotree3
Copy link

@timotree3 timotree3 commented Jun 10, 2020

Resolves #23

Notably, CowBytes<'a> is not publicly visible because it isn't re-exported at the crate-level like the other structs.

Follow-up work would be to add proper trait impls and documentation and make the type public.

I haven't bothered to learn serde_test and as such this introduces some untested code in the cowbytes.rs module. If that would block this PR I am happy to learn it and add the relevant tests.

type Value = CowBytes<'de>;

fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("bytes")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas for a better message here?

self.bytes
}

pub fn into_cow_bytes(self) -> Cow<'a, Bytes> {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas for a better name here?

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I think I would prefer to start simpler without a CowBytes wrapper type. I've implemented this a different way in #25.

@dtolnay dtolnay closed this in #25 Jun 11, 2020
@timotree3 timotree3 deleted the introduce-cowbytes branch June 11, 2020 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Allow Cow<'a, [u8]> to be deserialized even when visitor receives owned bytes.
2 participants