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 plugin options handling #23

Merged
merged 6 commits into from
Dec 12, 2019
Merged

Conversation

antongolub
Copy link
Contributor

Hi there.

I'd like to add optional plugin configuration. There are a couple of solutions:

  1. options as JSON (this PR)
--options={"plugins": [["pngquant", {"quality": [0.2, 0.5]}]]}
  1. options as properties
--plugin.webp.quality=95
--plugin.pngquant.quality.0=0.2 --plugin.pngquant.quality.1=0.5

Which looks better?

@sindresorhus
Copy link
Contributor

Definitely 2.

@antongolub
Copy link
Contributor Author

antongolub commented Aug 12, 2019

Done. There's also a useful side effect: now each plugin is loaded once.

Вefore:

imagemin --plugin=pngquant --plugin=pngquant --plugin=pngquant
↓
{
 ...,
 plugins: [
   require(`imagemin-pngquant`)(),
   require(`imagemin-pngquant`)(),
   require(`imagemin-pngquant`)()
 ]
}

After:

imagemin --plugin.pngquant.quality={0.1,0.2} --plugin=pngquant
↓
{
 ...,
 plugins: [
   require(`imagemin-pngquant`)({quality: [0.1, 0.2]})
 ]
}

@antongolub antongolub force-pushed the master branch 2 times, most recently from c1e1a58 to a07bff6 Compare August 12, 2019 08:47
@antongolub
Copy link
Contributor Author

@sindresorhus, any objections / suggestions?

@antongolub
Copy link
Contributor Author

antongolub commented Aug 30, 2019

@sindresorhus, is this good enough to be merged eventually? )

@antongolub
Copy link
Contributor Author

@1000ch, @kevva , could anybody review this PR?

This reverts commit 10f34bc.
@toufali
Copy link

toufali commented Dec 4, 2019

Eagerly awaiting this merge 🙏

@sindresorhus sindresorhus merged commit 615f664 into imagemin:master Dec 12, 2019
@sindresorhus
Copy link
Contributor

Thanks, @antongolub 👍

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.

3 participants