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 OOP Implementation #15

Merged
merged 28 commits into from
Jul 21, 2021
Merged

macOS OOP Implementation #15

merged 28 commits into from
Jul 21, 2021

Conversation

williamchange
Copy link
Contributor

This fixes/amend issues in darwin.py that prevents the addon from functioning correctly on macOS:

  1. Missing execute function calls
  2. Capture the first result from stdout/stderr

I also noted and fixed a typo in the preference panel, a missing space between if and the

Other issues:
I also noticed that images gets written inside the .blend(which failed), is it the intended behavior? (The image doesn't get saved as the directory doesn't exist):

i.e. image.png being saved to ..filepath/project.blend/image.png, as reported by the addon

However the image do get saved correctly if it was saved to other directories, though it should be noted that saving an image with the osascript will always return an error(but still saves the file correctly), thus the image will not get pasted:

ImagePaste: Cannot save image: ImagePaste-210720-160706.png<.../project.blend/ImagePaste-210720-160706.png> (*** Error creating a JP2 color space: falling back to sRGB)

@williamchange
Copy link
Contributor Author

williamchange commented Jul 20, 2021

So far what's working:

  1. Copying to clipboard
  2. Pasting image(one or more files) from Finder to Blender

@thanhph111 thanhph111 changed the base branch from main to oop-implementation July 20, 2021 07:38
@thanhph111
Copy link
Collaborator

Thank you very much for taking the time to do this.
I've change the target branch to oop-implementation to make the main clean before this become official.

I also noticed that images gets written inside the .blend(which failed), is it the intended behavior? (The image doesn't get saved as the directory doesn't exist):

i.e. image.png being saved to ..filepath/project.blend/image.png, as reported by the addon

It's actually a bug, it has been fixed with 2a99d8c that I've just commited. Can you merge it again to have the update.

@thanhph111 thanhph111 self-requested a review July 20, 2021 08:00
@williamchange
Copy link
Contributor Author

williamchange commented Jul 20, 2021

Thank you very much for taking the time to do this.
I've change the target branch to oop-implementation to make the main clean before this become official.

I also noticed that images gets written inside the .blend(which failed), is it the intended behavior? (The image doesn't get saved as the directory doesn't exist):
i.e. image.png being saved to ..filepath/project.blend/image.png, as reported by the addon

It's actually a bug, it has been fixed with 2a99d8c that I've just commited. Can you merge it again to have the update.

Yes I can confirm the files are saving correctly now, but still won't be pasted correctly(just one image in clipboard) as it will always return an error:

e.g. (*** Error creating a JP2 color space: falling back to sRGB)

, so now one of the ways to amend this is to see if files get saved(checking if file actually saved to the directory after running the command). What do you think?

@williamchange
Copy link
Contributor Author

I can confirm now that all of the features are working normally now.

@williamchange williamchange marked this pull request as ready for review July 20, 2021 09:09
@thanhph111
Copy link
Collaborator

Yeah, if not isfile(filepath): is a good check, but you mean this line still writes an image even there is an error.

e.g. (*** Error creating a JP2 color space: falling back to sRGB)

@williamchange
Copy link
Contributor Author

Yeah, if not isfile(filepath): is a good check, but you mean this line still writes an image even there is an error.

e.g. (*** Error creating a JP2 color space: falling back to sRGB)

Yeah I have tested it, the image still get correctly written to even though there's an error.

Co-authored-by: Thanh Phan <thanhph111@gmail.com>
Copy link
Collaborator

@thanhph111 thanhph111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's really better now, I just have a few things to ask

imagepaste/clipboard/darwin/darwin.py Outdated Show resolved Hide resolved
imagepaste/clipboard/darwin/darwin.py Outdated Show resolved Hide resolved
williamchange and others added 3 commits July 20, 2021 21:01
It's a change that has been removed from the `main`. Also rename the
variable and module to match the author's use case.
@thanhph111 thanhph111 merged commit 5c0e789 into b-init:oop-implementation Jul 21, 2021
@williamchange williamchange deleted the oop-implementation branch July 21, 2021 09:42
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.

3 participants