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 option to replace the original file with -f flag #76

Merged
merged 2 commits into from
Jan 20, 2020

Conversation

marcinkrzeminski
Copy link

@marcinkrzeminski marcinkrzeminski commented Jan 11, 2020

Hi,

with your code adjustments and a little bit of research, I was able to implement the -f, --force flag. By providing this flag as a first argument you can now replace the original file.

Feel free to refactor my code if it's crap ;). Like I said before I'm not a Python dev.

Examples:

Single file

crunch -f file.png

Directory recursive

find. -name "*.png" | xargs crunch -f

Tested on macOS Catalina, Version: 10.15.1 (19B88)

@chrissimpkins
Copy link
Owner

Thank you very much!! I will have a look and try to add tests this week so that we can get this merged. I really appreciate it!

@chrissimpkins chrissimpkins merged commit 1ba8aba into chrissimpkins:dev Jan 20, 2020
@chrissimpkins
Copy link
Owner

chrissimpkins commented Jan 20, 2020

I pushed these changes to the dev branch Marcin. Thank you very much for this work. I made some very minor edits to your source contribution in 02b5e45.

I still need to add new tests for the inplace file write functionality before we merge this to master. I am also thinking through how we might include this in the GUI and macOS services. Perhaps through a config file? Let's see.

I'd like to examine a handful of upstream changes that have happened in zopfli since our last release. I may try to roll that into a release that includes your new force/inplace overwrite support here.

Thanks again! We'll get this out soon!

@marcinkrzeminski
Copy link
Author

Thanks for letting me know :).

As of GUI, I'd suggest a checkbox if possible. Config file seems a bit cumbersome, especially for less experienced users which are more likely to use the GUI version, instead of the command line.

@chrissimpkins
Copy link
Owner

As of GUI, I'd suggest a checkbox if possible. Config file seems a bit cumbersome, especially for less experienced users which are more likely to use the GUI version, instead of the command line.

Absolutely agree. This will require a full rewrite of the GUI application. It is just an automated wrapper around the Python script at the moment and I don't have a way to implement the settings configuration. Looking into this.

@chrissimpkins chrissimpkins mentioned this pull request Oct 29, 2020
1 task
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