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 optional serialization using serde #5

Merged
merged 4 commits into from
Dec 5, 2020

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Dec 4, 2020

This PR is intended to fix #4 by making all structs serializable using serde when the new serde feature is used in the dependency declaration.

It also adds derive(Debug) to Webpage and WebpageOptions (as they were missing) and fixes some tests and a small bug.

It seems like the tests in schema_org.rs are using an old API that used an
Option as the return for SchemaOrg::from().
While filling up html.text_content in the parser, a leading space is added
unconditionally to separate multiple segments, but for the first segment the
space should be avoided.
When including the new features = ["serde"] in the dependency specification, all
structs provided by the library become (de)serializable using serde.
@llucax
Copy link
Contributor Author

llucax commented Dec 4, 2020

I just amended the last commit with some fixes. Now I tested it creating a simple test crate consisting of:

fn main() {
    let web = webpage::Webpage::from_url(
        "http://www.rust-lang.org/en-US/",
        webpage::WebpageOptions::default(),
    )
    .unwrap();
    println!("{}", serde_json::to_string(&web).unwrap());
}

And

[dependencies]
webpage = { path = "../webpage-rs", features = ["serde"] }
serde_json = "1.0"

To run the tests with the feature enabled, I had to use cargo test --features serde. If there is any better way to make sure this is tested by the tests, I'd be glad to add it.

@orottier orottier merged commit 6499ed6 into orottier:master Dec 5, 2020
@orottier
Copy link
Owner

orottier commented Dec 5, 2020

Thanks, also for the additional fixes!

@llucax llucax deleted the serialization branch December 5, 2020 21:42
@llucax
Copy link
Contributor Author

llucax commented Dec 5, 2020

Thanks for meeting it in so quickly!

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.

Serialization
2 participants