http-rs / http-types

Common types for HTTP operations
https://docs.rs/http-types
Apache License 2.0
200 stars 84 forks source link

`Body::set_mime` should take `Option<impl Into<Mime>>` #381

Closed yoshuawuyts closed 3 years ago

yoshuawuyts commented 3 years ago

It's currently impossible to unset the mime associated with the body. We should make it so we can unset the mime passed to the body.

jbr commented 3 years ago

I believe this doesn't work because the compiler can't infer the impl generic for None. We could have a Body::remove_mime, maybe?

halvko commented 3 years ago

@yoshuawuyts The code in the PR is finished (and correct), but I've shown in the doc comment of the function why it (as @jbr hints at) a somewhat ugly solution. As I write in the commit message, there are a couple of alternatives: Better documentation, or the introduction of a function.

There is also the possibility I've misunderstood you and you want Body::mime to be an Option, but I'm not familiar enough with the code base to know if that would make sense.

yoshuawuyts commented 3 years ago

There is also the possibility I've misunderstood you and you want Body::mime to be an Option, but I'm not familiar enough with the code base to know if that would make sense.

Yeah, that's what I was thinking 😅 . I ran into this when trying to write a converter for vcr-cassette where we couldn't express a body without a mime type. Good defaults are important, but having a way to actually unset the mime type seems like something we should support.

halvko commented 3 years ago

@yoshuawuyts #387 should do this correctly now, and doesn't add any new warnings

halvko commented 3 years ago

@yoshuawuyts As #387 was merged, I think this should be closed