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

Reexport URL #71

Merged
merged 1 commit into from
Aug 11, 2018
Merged

Reexport URL #71

merged 1 commit into from
Aug 11, 2018

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Aug 10, 2018

This makes using lanugageserver_types slightly more convenient.

@Marwes
Copy link
Member

Marwes commented Aug 10, 2018

Given how core Url is this seems sensible. Only thing is if it would be better to re-export the url crate instead (but we aren't really using anything other than Url)?

@matklad
Copy link
Contributor Author

matklad commented Aug 10, 2018

Only thing is if it would be better to re-export the url crate instead (but we aren't really using anything other than Url)?

Hm, don't think that instead would be better. It hypothetically might be useful to export url in addition to Url, but that can be done later, once someone actually needs this.

@Marwes Marwes merged commit d17c24a into gluon-lang:master Aug 11, 2018
@matklad matklad deleted the patch-1 branch August 11, 2018 07:44
@Marwes
Copy link
Member

Marwes commented Aug 11, 2018

Thanks! Released as 0.48.1

@matklad
Copy link
Contributor Author

matklad commented Aug 29, 2018

With more experience with this, it turns out that working with Url is rather annoying: you need url_serde for serialization, and it also uses () for errors, so ? does not work.

It might make sense to make a newtype wrapper around Url to avoid this, but it looks like a lot of work for not too much benefit.

@Marwes
Copy link
Member

Marwes commented Aug 29, 2018

Yeah, it is not the best :/ There is an (old) PR for it servo/rust-url#327 but unfortunately they seem to want to wait for serde-rs/serde#953

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.

None yet

2 participants