NikitaIvanovV / ctpv

Image previews for lf file manager
https://www.nikitaivanov.com/man1/ctpv
MIT License
272 stars 26 forks source link

Kitty backend doesn't work for kitty v0.21.2 #13

Closed NikitaIvanovV closed 2 years ago

NikitaIvanovV commented 2 years ago

Context: https://github.com/NikitaIvanovV/ctpv/issues/12

NikitaIvanovV commented 2 years ago

@mangkoran Fixed in 17392c53596f91bbbff2defd84ab8cd99d105c44, could you please check if it works?

mangkoran commented 2 years ago

Edit4: I can confirm that it runs very well. image

Edit3: I should be able to build ctpv only by using make ctpv. However, as I trace your Makefile, it seems that gen/ directory contents (helpers, previews, server) are built and saved to disk as root/root. Is this intended? image

NikitaIvanovV commented 2 years ago

I assume you previously build and installed the program by running sudo make install. Could you try running sudo make clean (effectively removing all generated files) and then make ctpv to see if files have proper ownership?

mangkoran commented 2 years ago

Unfortunately, now I have no sudo access (work PC).

In my opinion, it could be better if user have the option to whether install it system wide (root) or local user only. Changing below https://github.com/NikitaIvanovV/ctpv/blob/688703db50192c8bab393d8f8e77365f66ea08f0/Makefile#L1

to $HOME/.local (cmiiw) should be enough for the local user installation.

Edit: Or, the install script can detect whether its invoked as root or no, and set the install dir accordingly.

NikitaIvanovV commented 2 years ago

Unfortunately, now I have no sudo access (work PC).

If you don't have sudo access, how did those files end up being owned by root in the first place? :thinking:

In my opinion, it could be better if user have the option to whether install it system wide (root) or local user only.

A user already has this option! This is why I added the PREFIX variable. You can also just run make PREFIX=~/.local install, without editing the Makefile.

mangkoran commented 2 years ago

If you don't have sudo access, how did those files end up being owned by root in the first place? thinking

I had sudo access. I need to request to IT department beforehand, and I did install ctpv using sudo. Now my access has been revoked.

A user already has this option! This is why I added the PREFIX variable. You can also just run make PREFIX=~/.local install, without editing the Makefile.

My bad. However, I think it can be better if this info is also put in the readme (for better clarity. User need to check the Makefile first before they know they can install locally :D).

NikitaIvanovV commented 2 years ago

Thank you for suggestions!