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

macOS GUI fails with space in absolute filepath [FIX AVAILABLE FOR TESTING] #30

Closed
masey9 opened this issue May 16, 2018 · 19 comments
Closed

Comments

@masey9
Copy link

masey9 commented May 16, 2018

I noticed this evening that Crunch will only execute successfully when I drag in PNG files from my Mac desktop into the GUI. If I drag them in from a folder on the desktop (eg. desktop/folder/image.png) then crunching fails to execute.


@chrissimpkins edits below:

Fix available for testing in #38 for anyone who is affected by this issue. Please weigh in on whether this fixes it for you in this thread. Thanks.

@chrissimpkins
Copy link
Owner

Confirming that this is with v2.0.0 or v2.0.1?

@chrissimpkins
Copy link
Owner

chrissimpkins commented May 16, 2018

I can't reproduce this issue. Here is a test with Crunch v2.0.1 from a subdirectory in my Desktop directory on macOS 10.12.6:

src0s-video

If you are able to do so, try installing the command line executable and run the following inside the image directory:

$ crunch *.png

and if that works, change to the desktop directory and execute this command:

$ crunch folder/*.png

We will see error reporting with the CLI tool. The GUI application uses the same Python script that defines the command line executable. This will provide a bit of guidance on where to start with this issue. Sorry for the troubles.

@masey9
Copy link
Author

masey9 commented May 16, 2018

Definitely using v2.0.1.
error-example

I captured the error messaging here:
error-message

@chrissimpkins
Copy link
Owner

hmm... are you able to link those images in the repository thread here? not sure what is causing that.

@chrissimpkins
Copy link
Owner

chrissimpkins commented May 16, 2018

It is a problem with pngquant execution. can you confirm that you see the same issue from any directory? Also can you modify the file names and remove the @ number from the path? wonder if that is causing it.

@masey9
Copy link
Author

masey9 commented May 16, 2018

  1. Example set of images attached.
    example-original-pngs-from-sketch.zip

  2. Can confirm it occurs from any subfolder on desktop, on Dropbox etc.

  3. I can confirm that removing the @ number from the path does NOT fix the issue.

@chrissimpkins
Copy link
Owner

All files are valid PNG:

$ pngcheck *.png
OK: russia-678-home@2x.png (852x1608, 32-bit RGB+alpha, non-interlaced, 77.9%).
OK: russia-678-lock@2x.png (852x1608, 32-bit RGB+alpha, non-interlaced, 76.7%).
OK: russia-x-home@3x.png (1578x2889, 32-bit RGB+alpha, non-interlaced, 81.1%).
OK: russia-x-lock@3x.png (1578x2889, 32-bit RGB+alpha, non-interlaced, 79.0%).

No errors were detected in 3 of the 3 files tested.

@chrissimpkins
Copy link
Owner

And all process without issues with the command line executable:

$ crunch *.png
Crunching ...
Spawning 4 processes to optimize 4 image files...
[ 16.09% ] russia-678-home@2x-crunch.png (195089 bytes)
[ 22.33% ] russia-678-lock@2x-crunch.png (284852 bytes)
[ 22.19% ] russia-x-lock@3x-crunch.png (849628 bytes)
[ 14.15% ] russia-x-home@3x-crunch.png (487548 bytes)

@chrissimpkins
Copy link
Owner

chrissimpkins commented May 16, 2018

I can't reproduce this. I can verify that all four files compress for me through the DnD GUI:

ys066-image

I executed the pngquant and zopflipng command directly on each of the images from the pngquant and zopflipng that are embedded with the application package and I receive no errors. It executes without issues and all four files are compressed.

Mind executing the following command from your terminal inside the directory that contains those images?

$ /Applications/Crunch.app/Contents/Resources/pngquant  --quality=80-98 --skip-if-larger --force --ext -crunch.png *.png

Let me know if you get an error message.

@chrissimpkins
Copy link
Owner

Is there a space between POOL and F in the directory path?

@masey9
Copy link
Author

masey9 commented May 16, 2018

Yup, there is a space. I'll test without the space. One sec...

@masey9
Copy link
Author

masey9 commented May 16, 2018

Yup, that seems to have been the problem. No good on subfolders that have spaces in them. ☹️

@chrissimpkins
Copy link
Owner

OK. Thanks for checking. That is a simple fix. I am not escaping spaces in the absolute paths to the files. Will push a fix for this tonight.

@masey9
Copy link
Author

masey9 commented May 16, 2018

Happy days! Well done on finding that one. 👍🏼

@masey9 masey9 closed this as completed May 16, 2018
@chrissimpkins chrissimpkins reopened this May 16, 2018
@chrissimpkins
Copy link
Owner

Thanks for reporting it! Will leave this open until I definitely have a fix for it. Will post here when available.

@chrissimpkins chrissimpkins changed the title Will only crunch from Desktop macOS GUI fails with space in absolute filepath May 16, 2018
@chrissimpkins
Copy link
Owner

chrissimpkins commented May 17, 2018

Opened #38 with a fix for this. A dmg installer for v2.0.2-beta1 is available in this directory: https://github.com/chrissimpkins/Crunch/tree/path-space-escape-fix/installer

Mind letting me know if this fixes the problem for you? It wasn't as trivial as I initially thought but I think that we have a fix here. I have confirmed that the macOS GUI now works with spaces in the directory paths +/- spaces in the filename. This means that all of these should now properly complete execution as of the v2.0.2-beta1 build whereas they would not before:

/some/dir space/image.png
/some/dir/image copy.png
/some/dir space/image copy.png

@chrissimpkins chrissimpkins changed the title macOS GUI fails with space in absolute filepath macOS GUI fails with space in absolute filepath [FIX AVAILABLE FOR TESTING] May 17, 2018
@masey9
Copy link
Author

masey9 commented May 17, 2018

Ran the same test as last night and this morning. I can confirm it's working a treat now even if I have a space in the subfolder name (eg. POOL E/image.png). Thanks for taking care of that one Chris - big help as far as my workflow goes. 👍🏼

@chrissimpkins
Copy link
Owner

Excellent. Thanks for confirming that it is fixed and for taking the time to report this! I will get the patch out today.

@chrissimpkins
Copy link
Owner

Fix released as v2.0.2. Homebrew cask update PR submitted Homebrew/homebrew-cask#47349. The Homebrew cask release should be available in the next several hours.

Thanks for this report and your help with testing the fix @masey9!

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