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 quality param #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ddavisso4
Copy link

Was converting a bunch of files and was getting 11MB jpgs from 3MB HEICs. Turning quality down to 85% resulted in similarly-sized files with quality good enough for my use. Perhaps this will be useful to someone else.

@catdad
Copy link
Member

catdad commented Nov 14, 2021

This addressed #7

@catdad
Copy link
Member

catdad commented Nov 14, 2021

Hey, thanks for your interest in this! Would you be able to add some tests for this new functionality and update the readme, please, so that others may contribute with confidence in the future?

.check((args) => {
if (args.quality != null) {
if (args.quality <= 0 || args.quality > 1) {
throw 'quality must be a number between 0 and 1';
Copy link
Member

Choose a reason for hiding this comment

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

All values being thrown should be errors. This might even be a time where it's appropriate to use RangeError!

Suggested change
throw 'quality must be a number between 0 and 1';
throw new RangeError('quality must be a number between 0 and 1');

.option('quality', {
alias: 'q',
describe: 'The jpeg compression quality, between 0 and 1',
default: '1',
Copy link
Member

Choose a reason for hiding this comment

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

I am not entirely sure whether yargs typecasts the default value, but it will definitely typecast the parsed values. Since this is a number, it would be best that the default also be a number rather than a string.

Suggested change
default: '1',
default: 1,

const buffer = input === '-' ?
await readStdin() :
await promisify(fs.readFile)(path.resolve('.', input));

const results = await convert.all({ buffer, format: FORMAT[format], quality: 1 });
const results = await convert.all({ buffer, format: FORMAT[format], quality: quality });
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const results = await convert.all({ buffer, format: FORMAT[format], quality: quality });
const results = await convert.all({ buffer, format: FORMAT[format], quality });

This is a good note for me that I actually forgot to add this rule to eslint. I will do that separately.

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

2 participants