kornelski/pngquant

Usage of batch processing in README.md is misleading

Closed this issue · 6 comments

The README says:

batch conversion of multiple files: pngquant *.png

Let's try:

$ /tmp/pngquant/bin/pngquant --skip-if-larger --verbose --ext .png -f 'inputs/Bilder.toy/*'
inputs/Bilder.toy/*:
  error: cannot open inputs/Bilder.toy/* for reading
There were errors quantizing 1 file out of a total of 1 file.

Does not work.
Reason: The README relies on the fact that the tool is run from a shell which supports file globbing, but the tool itself does not use glob(3). Better replace with multiple filenames. As soon as I run this example from a non-shell, e.g., Java, Python, popen it will fail.

It doesn't work, because you've put '*' inside quotes. In quotes it means exactly this character, nothing else. Without quotes it gets expanded to multiple files.

It doesn't work, because you've put '*' inside quotes. In quotes it means exactly this character, nothing else. Without quotes it gets expanded to multiple files.

Have you actually understood what I have written? Please reopen. I don't expect you to change the code, but simply fix the documentation.

The documentation is correct. pngquant *.png as written in the documentation works fine, and pngquant '*.png' doesn't work due to how globbing works. The example you've shown here is the invalid one.

No, you ate confusing shell wildcard expansion either application level globbing. If disabled it won't work either: https://stackoverflow.com/a/11456496

If we're talking about unixish systems, then relying on shell expansion is normal. ls '*.png' behaves the same. * is a valid filename character, so calling glob on already-expanded arguments could expand twice. That is a non-standard behavior, and it could have unintended consequences.

Command-spawning APIs that pass arguments literally without going through shell do this intentionally. This allows tools to invoke pngquant with specific file names, and not worry that it will process other files. If pngquant applied glob itself, then callers that expect to pass a literal path would be required to escape glob characters themselves, and that is unusual.

If we're talking about unixish systems, then relying on shell expansion is normal. ls '*.png' behaves the same. * is a valid filename character, so calling glob on already-expanded arguments could expand twice. That is a non-standard behavior, and it could have unintended consequences.

Command-spawning APIs that pass arguments literally without going through shell do this intentionally. This allows tools to invoke pngquant with specific file names, and not worry that it will process other files. If pngquant applied glob itself, then callers that expect to pass a literal path would be required to escape glob characters themselves, and that is unusual.

Yes, that is correct and that is why the README is confusing. It implies to rely on shell globbing, not application globbing. I would just recommend change the docs to make it clear about globbing or not mention globbing (wildcard expansion) at all.