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

Produce an empty layout result for svg without width or height attributes #69

Merged
merged 5 commits into from
Nov 11, 2019

Conversation

jseppi
Copy link
Contributor

@jseppi jseppi commented Nov 8, 2019

If an input SVG is missing width or height attributes, mapnik.fromSVGBytes will result in an error, which in turn makes generateLayout and generateImage fail.

This PR adds error-handling logic for this situation to instead callback with a null result, allowing the other layout sprite or image result to still be produced with the remaining "good" SVGs.

This is essentially an addendum to #62.

cc @aparlato @samanpwbb

@jseppi jseppi requested a review from mapsam November 8, 2019 15:22
@jseppi jseppi self-assigned this Nov 8, 2019
@jseppi jseppi requested a review from samanpwbb November 8, 2019 22:04
Copy link

@samanpwbb samanpwbb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

@@ -117,6 +117,8 @@ function generateLayoutInternal(options, callback) {
}
mapnik.Image.fromSVGBytes(img.svg, mapnikOpts, function(err, image) {
if (err && err.message.match(/image created from svg must be \d+ pixels or fewer on each side/) && options.removeOversizedIcons) return callback(null, null);
// Produce a null result if no width or height attributes. The error message from mapnik has a typo "then"; account for potential future fix to "than".
if (err && err.message.match(/image created from svg must have a width and height greater (then|than) zero/)) return callback(null, null);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow well done anticipating a fix for then|than 💀 – this typo has been haunting mapnik users forever.

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "@mapbox/spritezero",
"version": "6.1.1",
"version": "6.1.2-dev",
"main": "index.js",
"description": "small opinionated sprites",
"author": "Tom MacWright",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@jseppi jseppi merged commit 1066639 into master Nov 11, 2019
@jseppi jseppi deleted the handle-no-width-height branch November 11, 2019 16:09
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.

2 participants