catimg
catimg copied to clipboard
Simplify width/height logic
Currently the logic behind -w and -H flags is not straightforward. Here are a few problems:
- The 2 flags cannot be used together and throw an error
- The units of -w and -H depend on the resolution. For example, passing -w 80 returns an image either 80 or 40 columns wide, depending on the resolution.
- Passing some value with -w returns a smaller image if the original itself has fewer pixels, i.e., there is no scaling up. Therefore one cannot treat -w flag as "return an image of this size" anyway.
- Some black magic was involved when -H flag was passed. I tried to analyze the code, but got lost in all the if statements. As one example, for some reason the resulting image can depend on whether
-H 0
is passed or not.
I propose to solve some of the problems above by simplifying the logic a bit. First of all, define the units of -w as columns and units of -H as lines. Second, treat -w and -H as maximally allowed width and height of the resulting image and use the terminal measurements if they are omitted. In other words, the resulting image should always fit into a box specified by -w and -H. This also allows one to use the 2 flags together without any errors.
What kind of change does this PR introduce? (check at least one)
- [ ] Bugfix
- [x] Feature
- [ ] Code style update
- [x] Refactor
- [ ] Build-related changes
- [ ] Other, please describe:
Does this PR introduce a breaking change? (check one)
- [x] Yes
- [ ] No
If yes, please describe the impact and migration path for existing applications: This PR intends to simplify the behavior of the program and technically some changes are breaking.
- First of all, passing a width 80 with resolution 2 now produces an image 80 columns wide, which is twice as large than before. The mitigation is simply halving the value for -w.
- Second, passing a single width flag now will not disregard the terminal height and vice versa. In order to allow overflowing simply pass in addition the height flag with a large enough number (or alternatively pass
-H -1
as this will overflow the unsigned int and produce a giant positive number). - Some other complex behaviors are now simplified (like passing
-H 0
), but I could not find a rational explanation for them to provide a migration path.
The PR fulfills these requirements:
- [NA] When resolving a specific issue, it's referenced in the PR's title (e.g.
fix #xxx[,#xxx]
, where "xxx" is the issue number) - [NA] All tests are passing
- [NA] New/updated tests are included
I did some testing in my own terminal. As the changes do not touch the actual img_resize()
logic and only manipulate the width/height values this should be enough.
If adding a new feature, the PR's description includes:
- [X] A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it) I hope my reason was convincing enough even without opening a PR =)
Nice! I will have to take a proper look in a few weeks