Method naming

[lnicola]

Continuing from #38.

Right now there are some get_mime_type and guess_mime_type methods, and the difference between them is that the former take an extension and the latter take a Path. There's also mime_str_for_path_ext from #38, which could have been called guess_mime_type_str.

Additionally,

• {guess,get}_mime_type return Mime
• {guess,get}_mime_type_opt return Option<Mime>
• get_mime_type_str and mime_str_for_path_ext return Option<&'static str>

To be honest, this is a bit of a mess. Can we settle on a naming convention that's more regular before releasing 2.0 (#42)?

[abonander]

Yeah, this is something I haven't been happy with myself, accumulating different functions that do slightly different things as multiple people had problems with different parts of the existing API.

This is what I'm thinking for 2.0:

• guess_mime, guess_mime_for_ext returns Mime
• mime_opt, mime_opt_for_ext returns Option<Mime>
• mime_str and mime_str_for_ext return Option<&'static str>

I'd also like to address #28 in this release by having a function that returns slices, but I dunno if that should just be mime_strs and mime_strs_for_ext returning Option<&'static [&'static str]> or if it should replace the functions just returning Option<&'static str> or what.

[lnicola]

Is there no way to reduce the number of combinations?

• for HTTP, the fallback in guess_ is a bad idea, but other applications might want it
• do we want both path and extension variants? Since the path variant doesn't do much, and can be misleading ("oh, it's probably opening the file and checking the first bytes")
• are both return types needed? Would returning a struct with a couple of methods work better? (mime_guess::from_extension("tif").or_fallback().as_mime())

[abonander]

I originally created this crate out of a need in the client API in multipart, where the Content-Type of a field is a hint at best so it doesn't have to be too accurate and application/octet-stream is the common fallback. That's where guess_* comes in, as well as the functions taking paths (just a convenience wrapper for extracting the extension).

I don't think functions taking paths is too misleading; it's extremely unidiomatic to hide disk access behind such an unassuming function that doesn't even return io::Result<_>. Barring that, it's explained very clearly (I think) in the documentation that those functions only work based on the path itself. Maybe worth renaming the path-functions to *_for_path?

I was considering a struct/method chain approach but I felt it was slightly more complex than this crate needs. I'd rather keep the API flat.

[lnicola]

I originally created this crate out of a need in the client API in multipart, where the Content-Type of a field is a hint at best so it doesn't have to be too accurate and application/octet-stream is the common fallback. That's where guess_* comes in, as well as the functions taking paths (just a convenience wrapper for extracting the extension).

I don't know about uploads, but on the server side it's better to not send a Content-Type at all -- I mentioned this in my PR, so I'm not going into that again.

I don't think functions taking paths is too misleading; it's extremely unidiomatic to hide disk access behind such an unassuming function that doesn't even return io::Result<_>.

Unless you expect the function to "guess" when there's an I/O error.

I was considering a struct/method chain approach but I felt it was slightly more complex than this crate needs. I'd rather keep the API flat.

Well, it's one solution to the O(N * M) proliferation of functions. It could also solve #28. It's okay if you don't like it -- the mime_guess API is pretty small in area, regardless of how you structure it -- so it doesn't matter too much. But it would fix these issues and make code more readable in my opinion.

[abonander]

Conversely, in multipart/form-data if a Content-Type is not specified then the server is supposed to assume it's text/plain, so application/octet-stream is the safer assumption if it's not a known text format: https://tools.ietf.org/html/rfc7578#section-4.4

Each part MAY have an (optional) "Content-Type" header field, which
defaults to "text/plain". If the contents of a file are to be sent,
the file data SHOULD be labeled with an appropriate media type, if
known, or "application/octet-stream".

This is where my choice of fallback came from. Since it doesn't work as well for the server-side I'm fine dropping it.

You touched on this already, but what about guess_* returning a wrapper type, but a simpler one than you suggested that behaves like Option:

/// The possible `Mime` values for the given path/extension or empty otherwise.
pub struct MimeGuess(pub &'static [Mime]);

impl MimeGuess {
    // all of these return the first value in the slice or their respective fallback
    // clones should be cheap if the `Mime` is compile-time constructed
    pub fn or_text_plain(&self) -> Mime { ... }
    pub fn or_octet_stream(&self) -> Mime { ... }
    pub fn or(&self, mime: Mime) -> Mime { ... }
    pub fn or_else(&self, or_else: impl FnOnce() -> Mime) -> Mime { ... }
    // bikeshed `to_opt`
    pub fn to_mime(&self) -> Option<Mime> { ... }
    // bikeshed `to_opt_str`
    pub fn to_str(&self) -> Option<&'static str> { ... }
}

The new release of mime should have a proc-macro that can construct instances at compile-time, which gives us the additional benefit of AOT validating the media types returned by this crate and also eliminating the runtime parsing cost.

What about just guess_from_path and guess_from_ext that both return this type? And the reverse-mapping functions can just return strings.

[lnicola]

Sorry, I kinda' dropped the ball here. Yeah, it sounds great. One use case for strings I can think of is interop with the http crate.

[abonander]

The new API of mime represents them internally as strings so the conversion is trivial. If Mime is going to be in the public API of this crate it might as well be the linchpin of it, but there is an argument to be made for dropping it entirely.