catimg icon indicating copy to clipboard operation
catimg copied to clipboard

add option for height offset fix #67

Open acxz opened this issue 2 years ago • 10 comments

in vertical scaling this accounts for used spaced in the terminal such as a prompt

This is a bug fix that resolves the display of scaling to the max terminal height. In reality the prompt get shown immediately after the image is displayed in the buffer, resulting in a view of a cutoff image. Adding an offset (optional) allows for users to change the effective max terminal height which their image is scaled against to render the complete image in their currently open terminal window.

This also fixes jarring updates from gifs. As if the displayed gif is larger than the display buffer, the gif is not played out smoothly in the terminal. Adding this offset allows scaling of the gif to fit the displayed buffer (this includes the user shell prompt) to ensure the smooth playback of the gif.

This can be treated as a feature request, however it primarily solves a vertical scaling display bug.

fixes #67

this issue could also be fixed by #69

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

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

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

  • [ ] Yes
  • [x] No

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

The PR fulfills these requirements:

  • [x] 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

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)

Other information: An alternative to catimg, chafa properly resolves this issue and scales according with user prompts. For a workaround and temporary solution, users can use chafa.

acxz avatar Feb 27 '22 21:02 acxz

maybe we could add something like this but I still think you should just pass the -H and compute the height outside of catimg

posva avatar Feb 28 '22 09:02 posva

While it is totally possible to do so, one reason I really like catimg is that it resizes to my current terminal width. My prompt will always display after catimg and it will always push up the image/gif. It could be a bit cumbersome for users to manually specify a new height with -H every time they resize their terminal. With an offset approach users can still use the built-in automatic resize feature and add whatever offset their prompt is.

acxz avatar Feb 28 '22 14:02 acxz

I'm still not sure this logic belongs inside catimg:

# times 2 because of high resolution displaying
catimg -H $(( 2 * $(tput lines) - 2)) image.png

If we add an option like this it should default to 1 as most prompts are 1 height but work with other prompts too. It should also have a long name option only. Let's give this more time.

posva avatar Feb 28 '22 14:02 posva

default to 1

That does make sense, added

It should also have a long name option only.

tbh, I was actually thinking about that but didn't add it since there was 1) no previous option as a long name and 2) didn't want to add any more processing than needed.

acxz avatar Feb 28 '22 15:02 acxz

@posva any updates on this?

catimg -H $(( 2 * $(tput lines) - 2)) image.png

What we are suggesting is not exactly this.

Imagine a case where I don't know if my picture is limited by my terminal width or my terminal height. If I specify an argument to -H then it will resize my image to -H. However, if I do not specify an argument than catimg will decided whether to resize based on my terminal height or my terminal width, whichever is appropriate.

Here is a showcase of what I mean: First example is with -H with an arg (-H $(( 2 * $(tput lines) -2 ))), second case is only with -H with-arg-H

With only -H no-arg-H

acxz avatar Mar 03 '22 01:03 acxz

a long name option only.

What would you suggest? Do you think --max-height-offset is a good name?

acxz avatar Mar 03 '22 01:03 acxz

@posva can you provide closure to this PR?

acxz avatar Mar 16 '22 00:03 acxz

I will take a look in a few weeks once I have the time

posva avatar Apr 05 '22 09:04 posva

Closing due to inactivity.

acxz avatar Jul 01 '22 18:07 acxz

I still haven't had the time but this is an interesting PR

posva avatar Jul 02 '22 07:07 posva