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

Export mimetypes #127

Open
HerrNaN opened this issue Oct 19, 2020 · 5 comments · May be fixed by #258
Open

Export mimetypes #127

HerrNaN opened this issue Oct 19, 2020 · 5 comments · May be fixed by #258

Comments

@HerrNaN
Copy link

HerrNaN commented Oct 19, 2020

As a developer using this library I would like to use these already defined mimetypes instead of matching on string literals so that if something changes I don't have to worry and I don't have to double check that I've written the right mimetype correctly.

Eg.

mime, _ := mimetype.DetectFile(file)
if mime.Is(mimetype.SVG) ...

instead of

mime, _ := mimetype.DetectFile(file)
if mime.Is("image/svg+xml") ...

However this seems like something pretty obvious so if there is any reason this isn't already implemented please explain?

@tomcruise81
Copy link

Agreed. Happy to create a PR to that extent!

@gabriel-vasile
Copy link
Owner

I'm open for discussion on this.
The reason why I'm not convinced this is a good change is I don't think the benefits out-weight the pollution from 150+ constants added in library scope.

arg 1.

use these already defined mimetypes instead of matching on string literals so that if something changes I don't have to worry

If I have to rename a mimetype, then I will use the old mimetype as an alias.
Code like:

if mime.Is("image/svg+xml") ...

will continue to work the same.

arg 2.

I don't have to double check that I've written the right mimetype correctly

Ok, this would help with compile time errors, but I'd argue in general, one needs to pay attention to what he writes when coding.

tomcruise81 added a commit to tomcruise81/mimetype that referenced this issue Feb 25, 2022
@coveralls coveralls linked a pull request Feb 25, 2022 that will close this issue
@tomcruise81
Copy link

The challenge with the current approach is that from a developer perspective, I need to explicitly know the full type/subtype of the mimetype I want to reference, rather than just it's generalized name. With the exporting of these, it will muddy the namespace scope a bit, but will make finding the given mimetypes substantially faster.

Additionally, instead of:

mime, _ := mimetype.DetectFile(file)
if mime.Is("image/svg+xml") ...

the code becomes:

mime, _ := mimetype.DetectFile(file)
if mime == mimetype.SVG ...

@tomcruise81
Copy link

tomcruise81 commented Feb 25, 2022

The alternative use-case that interests me more is something like:

contentTypeHeader := resp.Header.Get("Content-Type")
mime := mimetype.Lookup(contentTypeHeader)
if mime == mimetype.SVG ...

It's desirable to have in a library like this, since it is not going to be added to net/http - golang/go#31572

@tomcruise81
Copy link

@gabriel-vasile - Is this something you're considering? If so, do you want anything else done on the PR?

Thanks!

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 a pull request may close this issue.

3 participants