-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Update wl-paste handling and return None for some errors in grabclipboard() on Linux #7745
Conversation
src/PIL/ImageGrab.py
Outdated
"Nothing is copied", # wl-paste, when the clipboard is empty | ||
"not available", # wl-paste, when an image isn't available | ||
"cannot convert", # xclip, when an image isn't available | ||
"There is no owner", # xclip, when the clipboard isn't initialized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you describe how I can manually reproduce this "xclip, when the clipboard isn't initialized" error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can reproduce that by logging oit of gnome, and logging back in. Then run the xclip command. I can confirm this happens on fedora 39.
src/PIL/ImageGrab.py
Outdated
] | ||
if any(e in err for e in allowed_errors): | ||
return None | ||
msg = f"{args[0]} error: {err.strip() if err else 'Unknown error'}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you actually find a scenario where an error wasn't returned, meaning that you needed "Unknown error"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, but I would like to be as possible, since we are calling userspace programs which could change their behaviour anytime
You mean because |
Yes, that behaviour is not consitant with the macos/windows implementation of the function, so if it does break if the developer isn't careful.(i.e. doesn't put a try-except block all around it). With this PR the majority of x11 and wayland "non-errors" (i.e. clipboard empty, or not containg images) will return None like mac/win instead of raising some exceptions. |
src/PIL/ImageGrab.py
Outdated
"not available", # wl-paste, when an image isn't available | ||
"cannot convert", # xclip, when an image isn't available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I have text in the clipboard, I get a "not available" message.
So that seems like the "not available" comment should mention that it also applies to xclip?
Could you explain how to replicate "cannot convert"?
xclip.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhh, I get this result. I tried copying from multiple applications, gnome terminal, firefox, kitty, and they all respond with the "cannot convert" error...
Screencast from 2024-01-27 09-06-37.webm
I will look into the xclip code to be sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After further inspection, it seems that the version debian uses (and thus ubuntu) is different from the upstream one, and does indeed error out with fprintf(stderr, "Error: target %s not available\n", atom_name);
instead of fprintf(stderr, " cannot convert %s selection to target '%s'\n", selection_name, atom_name);
.
I will update the comment accordingly and take the opportunty to squash those typos commits :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simpified logic and made it more robust against edge cases ( see the `allowed_errors` list ). Doing error checking this way, makes the behaviour of this function for x11 and wayland platforms more silimar to darwin and windows systems. fix typo src/PIL/ImageGrab.py Co-authored-by: Ondrej Baranovič <nulano@nulano.eu> fix typo src/PIL/ImageGrab.py Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com> ImageGrab: \added debian edge case to comment
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
Simplified code
I've created nik012003#3 to test two failing wl-paste scenarios. |
Added test
Simpified logic and made it more robust against edge cases ( see the
allowed_errors
list ).Doing error checking this way, makes the behaviour of this function for x11 and wayland platforms more similar to darwin and windows systems.