Conversation
|
|
||
| impl<'a> From<&'a str> for Mime { | ||
| fn from(value: &'a str) -> Self { | ||
| Self::from_str(value).unwrap() |
There was a problem hiding this comment.
Could we use expect here instead? Probably copy over the message from FromStr's error.
There was a problem hiding this comment.
Not sure I follow - FromStr here doesn't have an error message itself, the message comes directly from parse, which would probably provide a more useful error than this would.
I suppose this may actually be better as TryFrom, but that does mean updating Tide internals to support it, and also, what would it do with such an error anyways?
I'm actually having a bit of 2nd thoughts on this, given our panic/internal unwrap discussion the other day -- do we really want to encourage people to write e.g. res.set_content_type("application/json")?
There was a problem hiding this comment.
do we really want to encourage people to write (...)
That's the idea; we already support this for status codes, headers, urls, and more.
There was a problem hiding this comment.
I meant, shouldn't we encourage people to use res.set_content_type(mime::TYPE), res.set_header(headers::HEADER, ...) instead, since that is more guaranteed to not have a name parsing panic?
There was a problem hiding this comment.
That aside though, what would expect() say that wouldn't already be more detailed from parse?
d73606c to
ca2f461
Compare
ca2f461 to
68c1866
Compare
|
@yoshuawuyts I don't know how this can be improved aside from making it |
Refs: http-rs/tide#575