xlucn / fontpreview-ueberzug

preview fonts in fzf
MIT License
163 stars 11 forks source link

Command line flag to resize preview window #7

Closed scrouthtv closed 3 years ago

scrouthtv commented 3 years ago

Hello, I added two minor things that really helped me (I have a small laptop screen and the default size is too big):

xlucn commented 3 years ago

The image should be scaled down to fit half of the terminal by ueberzug, thus the image size should be limited by the terminal size. I am not sure what is the issue you had. By "default size", do you mean the image size on screen (not resolution)?

scrouthtv commented 3 years ago

My entire desktop has 1280x780 pixels. If I run fontpreview, the first 6 or 7 letters are obstructed by the preview window: image I guess this is because of the left padding of the terminal

xlucn commented 3 years ago

This should be a bug. The image should only take half the terminal. I'll check it up. Could you tell me which version of fzf you are using?

scrouthtv commented 3 years ago

0.23.1

scrouthtv commented 3 years ago

I just tested removing the terminal padding, the preview still overlaps the fzf list

xlucn commented 3 years ago

Oh, I see what you mean by the padding, that might be indeed what caused this issue. I am not that familiar with how fzf determines the terminal size, and I lost it where the image resolution could fix this XD

xlucn commented 3 years ago

Could you roll back to commit e3618fb and try again (without terminal paddings perhaps)? (or you can simply revert what (edit:) the next commit does)

scrouthtv commented 3 years ago

~It's grown even bigger:~ Edit: The same: image

scrouthtv commented 3 years ago

I also have ueberzug 18.1.6 and termite 15

xlucn commented 3 years ago

Sorry XD, bad attempt.

I also have ueberzug 18.1.6 and termite 15

~I reproduced this with termite, I was using st, so it might be that fzf won't determine the terminal size in termite or something. You can try some other terminal emulators.~ edit: no sorry, forgot to refresh, I don't have the issue in termite either edit2: so can you scroll down to see if the new preview has the same issue?

scrouthtv commented 3 years ago

(I'm currently on head again) If I scroll down to another font, the issue persists: image

scrouthtv commented 3 years ago

I now removed all my config files for termite and the issue persists. You could try resizing your termite to the size I have and see if you get it as well.

scrouthtv commented 3 years ago

I now installed st with no patches v0.8.4 and it still overlaps

scrouthtv commented 3 years ago

Even with sh or bash in st it still overlaps.

xlucn commented 3 years ago

That is wired, I feel really confused now. I could not reproduce your issue with any window size in both termite and st. Maybe you can settle with you modification for now. If any of us figure out what the issue is, or some one else has the same issue, we'll then deal with it.

scrouthtv commented 3 years ago

Feel free to merge my branch nevertheless : )

dadav commented 3 years ago

I'm having exactly the same issue, the image overlaps with my fzf text!

img-2020-12-12-120158

My setup: kitty 0.19.2 zsh 5.8 i3-gaps 4.19 (gaps inner 10) resolution 1600x900

BUT: If I set gaps inner 0 the result looks like this: img-2020-12-12-120013

scrouthtv commented 3 years ago

With

ueberzug 18.1.6
i3-gaps 4.19
termite 15
zsh 5.8
resolution 1280x800
newest fontpreview-ueberzug

I still face this issue even with

gaps outer 0
gaps inner 0

But nice try ;)

xlucn commented 3 years ago

This might be a general problem, however I am current not able to figure out the reason. I will create a new issue and have the follow-up discussions there.

dadav commented 3 years ago

@scrouthtv could you try this pls? https://github.com/OliverLew/fontpreview-ueberzug/pull/11/commits/bfbf735ad05a84550c1c00941602bf9962029b89

xlucn commented 3 years ago

@dadav Thanks! That patch seems right. tput might be giving wrong geometry or something in your cases.

scrouthtv commented 3 years ago

It's working for me too!

xlucn commented 3 years ago

Great! I will then merge that PR and close this.