Add support for Serde 1.0

[dtolnay]

Fixes #304, fixes #320.

r? @SimonSapin

---

This change is 

[SamWhited]

LGTM; thanks for the comments on the copy/pasted code that could be derived, saved me trying to figure out why derive wasn't used.

[bors-servo]

☔ The latest upstream changes (presumably #331) made this pull request unmergeable. Please resolve the merge conflicts.

[dtolnay]

I fixed the conflict with #331.

[drusellers]

Looking forward to serde 1.0 support landing

[SimonSapin]

I find the code in src/serde.rs, especially Deserilize impls for enum, to be overly verbose. It sounds like extern crate serde; in the code generated by derive is the only reason we can’t use derive instead. I’ve filed serde-rs/serde#953 to request a serde feature that hopefully will fix that. I’d much prefer to use derive here.

[SimonSapin]

@drusellers In the meantime, https://docs.rs/url_serde/0.2.0/url_serde/ supports serde 1.0 today.

[bors-servo]

☔ The latest upstream changes (presumably #335) made this pull request unmergeable. Please resolve the merge conflicts.

[konstin]

Unfortunately, url_serde doesn't work on existing types, e.g. Vec<Url>, so using serde in my project is currently blocked by this PR.

[Object905]

@konstin you can use Vec<url_serde::SerdeUrl> instead.

[killercup]

I really wanted to do a cool demo in a talk where I fetch http://readrust.net/rust2018/feed.json, first show a nice list of posts, and as a bonus quickly extract the domain to see ho many people are using gist.github.com as their blog. Not having serde 1.0 support basically destroys that bonus feature :(

[SimonSapin]

@killercup See #327 (comment) above.

[bors-servo]

☔ The latest upstream changes (presumably #427) made this pull request unmergeable. Please resolve the merge conflicts.

[killercup]

@SimonSapin that's why I wrote "I really wanted to do a cool demo" 😉

Adding two crates and using

#[serde(with = "url_serde")]
url: Url,

is not nearly as cool as just writing url: Url,.

[SimonSapin]

Sorry that SemVer spaghetti is not cool :) I’m considering doing url 2.0 at some point to fix this and other things, but for Servo to use be able to update it’d first need to update hyper from 0.10 to 0.11, which seems to be blocked on websocket support: servo/servo#18518.

But since I’d rather not maintain two branches of url and I seem to be the one doing all that work anyway, url 2.0 will wait.

[killercup]

Ah, I see. That's indeed a pretty big chunk of spaghetti you have on your plate! I kinda thought using the serde09 crate would resolve this, but I guess the issues run quite a lot deeper than that.

…

Am 21.01.2018 um 21:04 schrieb Simon Sapin ***@***.***>: Sorry that SemVer spaghetti is not cool :) I’m considering doing url 2.0 at some point to fix this and other things, but for Servo to use be able to update it’d first need to update hyper from 0.10 to 0.11, which seems to be blocked on websocket support: servo/servo#18518. But since I’d rather not maintain two branches of url and I seem to be the one doing all that work anyway, url 2.0 will wait. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[dtolnay]

@SimonSapin I rebased and removed the 1.0 impls for Host<S> and HostInternal, because I believe most users are really only looking for Serde impls for url::Url. That one does not require serde_derive because we are serializing the url in string form.

[LegNeato]

Any update on this? Is this landable? Did something similar land?

[est31]

The 2.0 branch has been opened. Per prior statements, this PR should be closed and instead a PR against that branch filed.

I have made an alternative serde 1.0 implementation that's closer to the current implementation: 2.0...est31:serde_1.0

I think that this PR's serialization should be used instead where url's are just serialized as strings.

[bors-servo]

☔ The latest upstream changes (presumably #517) made this pull request unmergeable. Please resolve the merge conflicts.

[SimonSapin]

Thanks for the PR, David. The version looks more reasonable than the original.

I’m going to close this since I took #512 instead, to be part of url 2.0.0. Hopefully it’ll be easy enough for the ecosystem to migrate, but if not we can consider reviving this PR in the 1.x branch.