Improved wl-paste mimetype handling in ImageGrab#7094
Improved wl-paste mimetype handling in ImageGrab#7094hugovk merged 13 commits intopython-pillow:mainfrom
Conversation
|
I guess |
|
@nulano Thanks for your reply, I'll try to modify the GitHub workflow to make it work. |
Tests/test_imagegrab.py
Outdated
| im = ImageGrab.grabclipboard() | ||
| assert_image_equal_tofile(im, image_path) | ||
| except OSError as e: | ||
| pytest.skip(str(e)) |
There was a problem hiding this comment.
What is the OSError that might be thrown here?
There was a problem hiding this comment.
Oh, OSError will not be thrown here anymore. I made a mistake with skipif condition and OSError reported in other systems.
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
|
Hi everybody, I've tried a lot in test-workflow branch, but I couldn't find a solution to support wl-clipboard in xvfb. The errors include:
Do you have any suggestions? |
|
If you don't know of a simple way to add coverage for this, then I wouldn't think that is a blocker for this PR. You're not adding code that isn't covered into the middle of an otherwise-covered section - we weren't testing wl-paste or xclip on our CIs to begin with. See https://app.codecov.io/gh/python-pillow/Pillow/blob/main/src/PIL/ImageGrab.py |
|
Hi @radarhere , I finally found a way to test wl-paste by using sway. The CI is running, I'll commit to this branch if it works. |
|
Unfortunately, there is a problem. If I remove the fix, the test doesn't fail. |
|
Hi @radarhere , If you remove the fix, the test won't fail because there's only one MIME type (set by It seems impossible to pick several MIME types with |
|
I didn't set the CODECOV_TOKEN previously, but after setting it, I reran some workflows. |
|
I've created rrcgat#1 with some suggestions for this PR. |
Call init() if mimetype is not found with preinit()
|
Thanks for getting back to me! Apologies for the inconvenience, I wasn't aware of these suggestions until you left a comment. |
|
I've discovered a problem. I ran Ubuntu, switched to Wayland, opened https://python-pillow.org/images/pillow-logo-light-text-1280x640.png in Firefox, copied the image, and ran png.mp4If the first suggestion from I later tried copying a GIF from FIrefox. gif.mov |
|
@radarhere Yes, you're right, I didn't test it to much because I simply thought In the early days before, I found that When I opened https://python-pillow.org/images/pillow-logo-light-text-1280x640.png, Firefox and Chrome offered different MIME types (Chrome only offered It seems that there are too many cases of copy, iterate through the mime types works (with different MIME types, |
|
Use the hard way here: https://github.com/rrcgat/Pillow/pull/2/files What do you think? def grabclipboard():
...
else:
if shutil.which("wl-paste"):
args = ["wl-paste"]
output = subprocess.check_output(["wl-paste", "-l"]).decode()
clipboard_mimetypes = set(output.splitlines())
Image.preinit()
available_mimetypes = set(Image.MIME.values()) & clipboard_mimetypes
if not available_mimetypes:
Image.init()
available_mimetypes = set(Image.MIME.values()) & clipboard_mimetypes
if available_mimetypes:
return _grab(
*[["wl-paste", "-t", mimetype] for mimetype in available_mimetypes]
)
elif shutil.which("xclip"):
args = ["xclip", "-selection", "clipboard", "-t", "image/png", "-o"]
else:
msg = "wl-paste or xclip is required for ImageGrab.grabclipboard() on Linux"
raise NotImplementedError(msg)
return _grab(args)
def _grab(*call_args):
fh, filepath = tempfile.mkstemp()
for args in call_args:
subprocess.call(args, stdout=fh)
if os.path.getsize(fh):
break
os.close(fh)
try:
im = Image.open(filepath)
im.load()
finally: # Remove it anyway or left it when Image.open fails (for debug usage)?
os.unlink(filepath)
return im |
|
I think iterating through all of the different mime types doesn't sit nicely with #7110, a request that we raise an error on a failed ImageGrab operation - because if we try 12 different wl-paste variations, then we don't know which error to show. Let me suggest this instead. Given the output of
What do you think? |
|
@radarhere I agree, this approach is better than iterating through all mime types and it works well in my initial case. I will implement this solution based on your suggestion. |
…se the first mime type taken
|
Using the first mime type when there are multiple mime types without image/png is great, although image/png mime type is provided in the video: Screencast.from.2023-05-23.18-50-35.webm |
|
Thanks, looks good to me. I've created rrcgat#3 with minor formatting suggestions. |


Fixes #7093
Changes proposed in this pull request:
-t <mime/type>arguments towl-pastewhen there is an available image MIME type.