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

Allow specifying a format with captureScreen #293

Merged
merged 3 commits into from
Jan 19, 2025

Conversation

erjiang
Copy link
Contributor

@erjiang erjiang commented Jan 4, 2025

This allows it to be used with file handles, otherwise pillow doesn't know what format to write.

e.g.:

    buffer = BytesIO()
    # Capture screen to memory buffer in PNG format
    client.captureScreen(buffer, format="png")

This allows it to be used with file handles, otherwise pillow doesn't
know what format to write.
@Neustradamus
Copy link

@sibson: Have you seen this @erjiang PR?

@sibson
Copy link
Owner

sibson commented Jan 18, 2025

Thanks, @erjiang. Could you document what formats are valid and include a test case to ensure we don't regress this functionality.

I don't think the usage of format are very discoverable, even setting the default to 'png' or whatever PIL defaults to, would make it more clear. An alternative might be to name it image_format or file_type.

@erjiang
Copy link
Contributor Author

erjiang commented Jan 18, 2025

I don't think the usage of format are very discoverable, even setting the default to 'png' or whatever PIL defaults to, would make it more clear. An alternative might be to name it image_format or file_type.

I think this is mostly because of using Pillow - the list of formats is likely to change depending on the Pillow version and we don't want to set a default because the default should be to guess based on filename. Added some documentation to point to the Pillow image format list.

@sibson
Copy link
Owner

sibson commented Jan 19, 2025

👍

@sibson sibson merged commit ec3f481 into sibson:main Jan 19, 2025
10 checks passed
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