fix #30 Fit vertically/adjust to terminal height

[boretom]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing
• New/updated tests are included

If adding a new feature, the PR's description includes:

• This PR allows to adjust the output to terminal height instead of terminal width. The default behaviour stays the same only if you pass -H it changes. Passing -H or -H 0 will adjust the image to the terminal height, passing a number is adjust to that height.

[boretom]

Just as I submitted the PR I realized that I haven't considered both -w and -H passed. I only ever used one or the other. So you may want to hold on looking at that PR.

[posva]

Thank you for this!
Yeah, I think having both -w and -H could yield an error

[boretom]

Morning @posva, what behaviour would you prefer if both -w and -H are passed? I see the following options

1. only use whatever parameter is passed first, ignore the second
2. only use whatever parameter is passed last, ignore the one before
3. abort with error message, "only either -w or -H parameter is allowed"
4. allow both at the same time and resize width and height

Personally I would prefer number 3

[posva]

Aborting with an error message sounds good 👍

[boretom]

Hi, I added the check and abort if both are passed.

Additionally I did reorganise the man page file a bit so the options are sorted alphabetically. Plus reorganised the synopsis a bit. If you don't like it I'll of course restore the old man page + add the -H option.

Related to the man page I got some questions:

• are -t and -c arguments which can be used together?
• in -w it's written: "[...] by default catimg will use the terminal width". What does that mean?
• would it make sense to add an EXAMPLE section? To clarify that, eg. -w without a positiv integer is not allowed - although no error is printed right now. So even -w -r 1 works kind of but not really. In that case -r 1 is not working because -r is assigned as value to -w. Because -w expects an argument.

[posva]

Thanks a lot for this!

I need to test it a bit more to make sure it works on all machines. Where did you test so far?

[boretom]

I did test it on macOS 10.15.5, macOS 10.14.6, Linux Mint 19.3, Linux Mint 20, Raspberry Pi 3B+ with Debian Buster and FreeBSD 12.1.

[boretom]

Oh, and no hurry at all. I'm old enough to know that the priorities can vary greatly.

[posva]

Oh wow, you already tested it in so many devices. After the needed changes, it should be good for a merge!

[boretom]

Testing is a big word :) ... it compiles and works fine for me on the mentioned platforms/distros with different images and two animated gifs.

I haven't done any testing on Windows. It would be interesting because according to StackOverflow getting Y the way I did it (same as for X) gives the buffer height but not the terminal window height.

Btw: It also compiles on Haiku OS R1/Beta2 but the output in the BeOS Haiku OS terminal doesn't look right. But if I connect to the Haiku box using SSH from my macOS it looks correct. So it's the Haiku OS terminal which behaves badly. I'll drop a note to one of their mailing list but not this week.

Btw2: It compiles and works on Fedora 32 (all the OS except macOS & FreeBSD run on a Intel NUC) but you would have to move two variable definitions (color_map and yuv_color_map) out of sh_color.h to sh_color.c. Otherwise the linker complains about them being defined twice (I read one should define variables in header files for this reason). One of them - yuv_color_map - is defined again in sh_color.c so the one in sh_color.h can be deleted.
If you like I can make that change too.

[boretom]

I made the changes as I mentioned in the comments + what I guessed does "put span around math operators" mean.

[posva]

This looks good, thanks a lot!
I will have to take a look at upgrading through home-brew and others. I haven't done it in a while!

[posva]

I realized there is a floating point exception when providing a -H option that is too big. It should not exceed original height like it does with original width. I won't be able to take a look for some time, so please feel free to take a look at it and submit a PR!

[boretom]

I see, I'll check on it and great a PR. The same does not happen with -w?
What number did you use to test (I tested with -1 and 100000) and on what OS?

[boretom]

And regarding upgrading homebrew. a) no hurry and b) brew edit catimg shows you the recipe for that package.

[boretom]

I pulled the merged master and also get an floating point exception. But not with the feature/add-height branch, strange. I'll investigate it this week.