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

keep aspect ratio for thumb #216

Closed
eworm-de opened this issue Jan 26, 2023 · 6 comments · Fixed by #217
Closed

keep aspect ratio for thumb #216

eworm-de opened this issue Jan 26, 2023 · 6 comments · Fixed by #217

Comments

@eworm-de
Copy link

I use to run this command to capture the screen, including a thumb:

scrot --thumb 100x0 /tmp/screen.png

The thumb used to have a width of 100 pixels, preserving the correct aspect ration. This broke with ff88b0b (I think, to be verified...) and returns an error message now:

scrot: option --thumb: resolution height '0' is too small

Can we define a special case for value 0 and have the old behavior back?

@guijan
Copy link
Contributor

guijan commented Jan 26, 2023

I've just tried the example invocation with both commit ff88b0b and the preceding ae4deba, and indeed, the change in behavior was introduced by ff88b0b.

@eworm-de
Copy link
Author

Will you take care of this or should I have a look myself?

@guijan
Copy link
Contributor

guijan commented Jan 26, 2023

Yes, I'll fix it.

I'm about to go to sleep but I'll be adding this feature back and documenting it (it was undocumented), hopefully there will be a PR today.

Did you find out about it by reading the source code?

@eworm-de
Copy link
Author

Found out what, that it works or that it breaks? 😜

Other tools support this or something similar, for example imagemagick:
https://imagemagick.org/script/command-line-processing.php#geometry

So I did just try and it worked.

Well, now it broke. 😝

@daltomi
Copy link
Collaborator

daltomi commented Jan 26, 2023

Too bad, I did my best to test that PR :/
For the next one I must be more strict so that the changes in a PR do not cover more than what is necessary.

@eworm-de
Copy link
Author

Do not worry! This functionality was undocumented, so there was no reason for you to test it.

Nevertheless it would be nice to have the behavior back.

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 a pull request may close this issue.

3 participants