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

Add php mime type #7

Merged
merged 3 commits into from
Jul 5, 2019
Merged

Add php mime type #7

merged 3 commits into from
Jul 5, 2019

Conversation

mallardduck
Copy link
Contributor

While PHP files don't have an official mime type adding these in will allow the package to identify various php extensions. The type used is one prefixed with text since PHP files should be human readable, this allows someone to identify a PHP file as plain text.

It's particularly useful to detect and then display the contents of a PHP file in a web app.

@mallardduck
Copy link
Contributor Author

So I started looking into why this fails...and I got really confused.
I'm still mostly confused but I think I understand.

In the testDefaultDumpExtensionToType test I dropped a var_dump before and after the assert statement. So it looked like:

        var_dump(count($dump));
        $this->assertCount(988, array_keys($dump));
        var_dump(count($dump));

When running PHPunit this produced:

............../home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:92:
int(988)
/home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:94:
int(988)
........F/home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:92:
int(981)
........F/home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:92:
int(981)
.........../home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:92:
int(988)
/home/dan/Projects/dflydev-apache-mime-types/tests/Dflydev/ApacheMimeTypes/AbstractRepositoryTest.php:94:
int(988)
....                   47 / 47 (100%)

So I guess that means there's something going on in the test (or the code when in testing) that I don't understand. Oddly enough I was able to fix the failure by changing it to:

$this->assertCount(count($dump), array_keys($dump));

The test starts passing; i just don't know how valid of a test that is with this behaviour I can't explain.

@mallardduck
Copy link
Contributor Author

Should see comments above for more info before merging......

........but this should be ready to ship. This does remove tests for older PHP versions that are considered End of Life.

I'm not sure if that's something you'd like to do @simensen; at the least tho it seems like dropping 5.3.x should be done.

@spotman
Copy link

spotman commented Feb 11, 2018

+1 for merging this!

@simensen
Copy link
Member

Could you try something like:

$keys = array_keys($dump);
$this->assertCount(988, $keys)

... maybe it is something with how array_keys is being used? Curious to know if that makes a difference. Also, is it failing for all versions of PHP or just some of them?

I'm happy to drop older versions of PHP. :)

@mallardduck
Copy link
Contributor Author

Sorry on the terrible follow up. I will look into this again tonight and shoot an updated PR over.

@mallardduck
Copy link
Contributor Author

This should be all set for merge now!

@simensen
Copy link
Member

@mallardduck Now it is my turn for horrible follow-up. I just merged a few other PRs that have been open for awhile. It looks like you have put some work into two separate PR's that both now conflict. If you're up for it, I can probably merge a new PR pretty quickly. If I am not responsive on GitHub, feel free to find me on Twitter and pester me there.

@mallardduck
Copy link
Contributor Author

Absolutely! I will refactor this tonight and shoot the update over ASAP.

@mallardduck
Copy link
Contributor Author

@simensen Good to go!

While PHP files don't have an official mime type adding these in will allow the package to identify various php extensions. The type used is one prefixed with text since PHP files should be human readable, this allows someone to identify a PHP file as plain text.

It's particularly useful to detect and then display the contents of a PHP file in a web app.
...PHP 5.5 should probably go too, but I can see a reason to keep it.
...I hadn't update the other files which caused tests to be weird. Also undoes the sill change I made to hack tests into working.
@mallardduck
Copy link
Contributor Author

@simensen Just redid my refactor - the prior one was just a quick/dirty rebase. This one is a lot more concise.

@simensen simensen merged commit 8fc46f8 into dflydev:master Jul 5, 2019
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 this pull request may close these issues.

None yet

3 participants