Add support for .ico files

nodeca / probe-image-size

Get image size without full download. Supported image types: JPG, GIF, PNG, WebP, BMP, TIFF, SVG, PSD, ICO.

MIT License

978 stars 77 forks source link

Add support for .ico files #42

Closed leolabs closed 3 years ago

leolabs commented 3 years ago

This PR adds support for .ico and .cur files, which both use the same file structure.

When a given .ico/.cur file contains multiple images, then the largest image's size is returned.

leolabs commented 3 years ago

Thanks for your quick review! I've made some changes based on your comments and the PR should now be ready to be merged. You can squash all commits into one during the merge using the menu next to the merge button:

puzrin commented 3 years ago

You can squash all commits into one during the merge using the menu next to the merge button:

I know, but i'd like to see CI results before merge. Because of suspisious tests failure https://travis-ci.org/github/nodeca/probe-image-size/jobs/739931912. I also disabled one tricky test in master and changed node versions. That's why i asked to rebase.

In general, everything should be fine now.

leolabs commented 3 years ago

Ah, alright. I've rebased it :)

puzrin commented 3 years ago

npm run coverage - see report.

Could you improve tests coverage for your code? See jpeg tests for example, how to write test vectors with ease (instead of real files).

leolabs commented 3 years ago

I've improved the code coverage to be better :)

puzrin commented 3 years ago

mmmm...PR?

puzrin commented 3 years ago

See https://github.com/nodeca/probe-image-size/blob/master/test/formats.js#L194

Since it's not convenient tot create real files for all edge cases, we create fake data instead.