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.

Sign up for GitHub

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

Jump to bottom

validate zoom levels #25

Closed
wants to merge 3 commits into from
Closed

validate zoom levels #25

wants to merge 3 commits into from

Conversation

ungoldman
Copy link
Contributor

This catches invalid zoom levels and exits the CLI with an informative error message.

$ ./bin/cli.js -z 0
Invalid zoom: 0. Must be 1-5.
$ ./bin/cli.js -z 6
Invalid zoom: 6. Must be 1-5.
$ ./bin/cli.js -z 4 -i
Invalid zoom for infrared: 4. Must be 1-3.

fixes #20

@ungoldman
Copy link
Contributor Author

I've also updated the defaults in bin/cli.js based on what was in the readme, and updated that section of the readme to match the output from himawari --help.

@jakiestfu
Copy link
Owner

This error validation should probably be happening inside of the script itself instead of the CLI, no?

@ungoldman
Copy link
Contributor Author

@jakiestfu the current PR only supports the CLI use case described in #20 where one would expect an error and process.exit(1) in the case of an invalid zoom.

How should that be handled in the programmatic version? I don't think it should be exiting the process, and right now errors are only set up to be caught by the error option.

  • Should an invalid zoom send an error message to options.error()?
  • Should the function return an error or false or something like that or just keep going?
  • Should it still default to 1 if the zoom is out of range?

@ungoldman ungoldman closed this Jun 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

--zoom is ignored and defaults to 1
2 participants
@ungoldman @jakiestfu