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

--recursive flag trys to compress non png files #547

Closed
LuckyTurtleDev opened this issue Aug 5, 2023 · 6 comments · Fixed by #548
Closed

--recursive flag trys to compress non png files #547

LuckyTurtleDev opened this issue Aug 5, 2023 · 6 comments · Fixed by #548

Comments

@LuckyTurtleDev
Copy link
Contributor

The --recursive flag, does also try to compress non png files (even it is not even an image). Is their any reason, why the files are not filtered by .png extension? This should speed up the recursive flag, since non png files need not be considered.

I can implement this, if this feature is wanted.

@andrews05
Copy link
Collaborator

This seems very sensible, I guess it's just never come up before. Feel free to open a PR 🙂

@TPS
Copy link

TPS commented Aug 6, 2023

@LuckyTurtleDev If you are implementing this, maybe consider an option to retain the current behavior, as well?

@andrews05
Copy link
Collaborator

Note that regardless of how --recursive works, you can always use globbing to do the same thing. ** for everything, **/*.png for just png files. So we really don't need two options for it.
(I realise this kind of globbing may be foreign to Windows users, but I'm pretty sure it does work in oxipng)

@XhmikosR
Copy link
Contributor

XhmikosR commented Aug 6, 2023

The problem is that due to the Windows globbing issues, --recursive is still the only way to recursively optimize folders. At least up until #536, which seems to work (I tested the build from CI PR).

@TPS
Copy link

TPS commented Aug 6, 2023

Globbing is brittle on Windows (e.g., search "glob" in this repo's issues/PRs/commits & you'll see what I mean), b/c it's not a shell/system function, so switches like this are preferred there.

@LuckyTurtleDev
Copy link
Contributor Author

LuckyTurtleDev commented Aug 6, 2023

I have impl this now and include a flag to keep the old behavior.

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 a pull request may close this issue.

4 participants