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

Handle unknown background formats gracefully #174

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

argv-minus-one
Copy link

One of the image formats supported by Finder for the background image is, somewhat surprisingly, PDF. That's a vector graphics format, so this could be used to make a single background image that scales cleanly to any screen resolution, without having to make a separate @2x version of the image.

Problem: appdmg chokes when trying to use a PDF as the background image, because image-size doesn't understand that format (and PDF doesn't have a pixel size anyway).

This PR does two things to help with that problem:

  • Skip calling image-size if a window.size is given in the JSON specification file.
  • When calling image-size fails, give a helpful error message that explains what actually happened (couldn't read the image size) and how to avoid it (by giving a window.size).

The dimensions of the background image are used as a default window size if the specification does not contain a `window.size` object.

Skipping this is useful because reading the background image's dimensions won't work if it's in a format that `image-size` doesn't understand, particularly PDF.
When reading the background image's dimensions fails, appdmg now tells the user that the error can be avoided by giving `window.size` in the JSON specification.
@sindresorhus
Copy link

sindresorhus commented Dec 6, 2019

It would also be useful to update the readme to mention that PDF is supported as background.

Here:

background (string, optional) - Path to your background

And here:

https://github.com/LinusU/node-appdmg#retina-background

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