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 serde support #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add serde support #41

wants to merge 2 commits into from

Conversation

Vitroxyn
Copy link

Addresses #37

Add implementations of Serialize and Deserialize for UniCase and Ascii based on lawliet89/unicase_serde.

As proposed by the rust api guidelines data structures should implement Serde's Serialize, Deserialize.

Serde is added as optional dependency which can be used as feature so that no breaking api changes are introduced.

@Vitroxyn
Copy link
Author

Vitroxyn commented Mar 1, 2020

Job #134.4 failed because serde_derive requires a minimum rustc version of 1.31.

@lawliet89
Copy link

lawliet89 commented Jul 14, 2020

I implemented this before in #21 but @seanmonstar wrote in that PR why he didn't want it merged.

I agree with other posters that due to serde-rs/serde#723, this is a very significant omission. It leads to a very poor user experience, especially for first time Rust users (cf. lawliet89/unicase_serde#1).

I urge @seanmonstar to consider merging this. I would happily deprecated my unicase_serde crate when this is supported out of the box.

@@ -74,6 +74,7 @@ use self::unicode::Unicode;

mod ascii;
mod unicode;
pub mod serde;

Choose a reason for hiding this comment

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

Suggested change
pub mod serde;
mod serde;

I'm not seeing why this needs to be public. Trait implementations aren't part of a module.

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.

3 participants