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

ImageOptim hook autocorrects #346

Closed
deivid-rodriguez opened this issue Feb 19, 2016 · 4 comments
Closed

ImageOptim hook autocorrects #346

deivid-rodriguez opened this issue Feb 19, 2016 · 4 comments

Comments

@deivid-rodriguez
Copy link

I've noticed that the ImageOptim hook autocorrects.

I wouldn't expect this since it's the only hook I've seen that changes the repository state. Right?

@sds
Copy link
Owner

sds commented Feb 19, 2016

You would be correct, but it's the nature of the tool itself. It at least doesn't stage those corrections.

If image_optim provided a way to see if an image was optimizable without optimizing it, then this wouldn't be a problem.

I'm open to suggestions of how to deal with this, but at the time it was deemed "not a big deal" as the user could always git checkout the original images. What's nice about having it autocorrect is you get the warning and then you can promptly stage the changes to get the optimized image.

In regards to autocorrection in general, I'm not against it, it's just not supported in the general case yet (see #238).

@deivid-rodriguez
Copy link
Author

@sds As you said, no big deal. But did the cope behave previously as one would expect (71b31bd)? Haven't tried it but from image_optim's README, the non-bang version of optimize_images doesn't seem to change the source image.

@sds
Copy link
Owner

sds commented Feb 20, 2016

Ah, yes, the hook used to call the Ruby API directly. I switched it in 71b31bd to use the executable since I didn't want to have to install the image_optim gem just to test Overcommit. Unfortunately, there was no mechanism to check if an image was optimizable via the command line at the time without modifying it.

I'm definitely open to a pull request that fixes this to be non-destructive. One solution could be to copy the applicable files to a temporary directory and run image_optim against those, reporting any files that were modified. Another would be to submit a patch upstream to support non-destructive optimization checks.

@sds
Copy link
Owner

sds commented Jun 25, 2020

Closing this ticket due to age. Happy to merge a pull request if this is desired!

@sds sds closed this as completed Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants