-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Mime] handle passing custom mime types as string #36716
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
Conversation
mcneely
commented
May 5, 2020
Q | A |
---|---|
Branch? | 4.4 |
Bug fix? | yes |
New feature? | no |
Deprecations? | no |
Tickets | Fix #36715 |
License | MIT |
Doc PR | none |
Fix's issue where custom mimetypes were failing |
I am not sure that this qualifies as a bug fix. Reading the code it clear that the constructor expects a list of extensions not a single one. Maybe we should clarify that by adding a docblock? Like this: /**
* @param array<string, array<int, string>> $map
*/
public function __construct(array $map = [])
{
// ...
} |
@xabbuh |
What I wanted to say is that this was probably not supposed to work: $mt = new MimeTypes([
'text/bar' => 'foo',
]); It should have been written like this instead: $mt = new MimeTypes([
'text/bar' => ['foo'],
]); |
What's wrong with adding that in? |
There's nothing wrong with it. I am just questioning whether this has been an intended usage. Depending on the answer this patch would either be a bugfix or a new feature. |
@xabbuh I have removed the line of convenience code you're so caught up on. Now it's only a bug fix. |
@nicolas-grekas I think the 7.1 integration just needs to be rerun, some sort of hiccup, probably because of 2 commits back to back. |
At least there is a bug that if you pass a string, it explodes. |
I misunderstood the comment I replied to. I understand the issue now and the proposed change fixes it. 👍 |
@@ -51,7 +51,8 @@ public function __construct(array $map = []) | |||
$this->extensions[$mimeType] = $extensions; | |||
|
|||
foreach ($extensions as $extension) { | |||
$this->mimeTypes[$extension] = $mimeType; | |||
$this->mimeTypes[$extension] = $this->mimeTypes[$extension] ?? []; | |||
array_push($this->mimeTypes[$extension], $mimeType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can shorten the patch a bit by using []
:
$this->mimeTypes[$extension][] = $mimeType;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with a minor change
3b84d0d
to
0921423
Compare
Thank you @mcneely. |