emsquid / pic

Preview Image in CLI.
MIT License
57 stars 4 forks source link

Feature/multiple preview #6

Closed Maratripa closed 1 year ago

Maratripa commented 1 year ago

Feature

Allow the use of wildcards to display more than one image.

Examples

pic -p iterm ~/Pictures/*.jpg

to display all .jpg images inside the Pictures directory.

pic ./*/*.png

to display all .png images inside any directory with depth 1 relative to pwd.

TODO

emsquid commented 1 year ago

That's great work, how do you think this should work when specifying arguments as --xand --y ? Won't all images get on top of each others ?

Maratripa commented 1 year ago

Just tested, -x does not affect as it just moves all images by an offset. -y does make the images overlap each other. What do you think would be the intended result here?

For example, if I pass the flag -y 3, what should happen?

emsquid commented 1 year ago

Imo it should work as spacing, but as you said it may be more annoying to implement... Btw thanks for your contribution I appreciate it a lot, I'm not participating too much as of now because I have exams next week, but I will be there more often once I am done :)

Maratripa commented 1 year ago

With more testing I found that kitty protocol does not work. When displaying an image, the previous one goes away. Also I think we could disable the -y flag for multiple preview and add a --spacing flag.

I'm in holidays, so I have plenty of time :D, plus: south hemisphere so i my semester starts on march :p

emsquid commented 1 year ago

Sounds good to me, for kitty protocol to work as you intend we would need to use different IDs, or we could use the uppercase I flag if I am not wrong, I know a bit about it so I should be able to deal with it

Maratripa commented 1 year ago

I don't fully understand yet how kitty protocol works, but maybe because the tempfile is overwritten every time the function is called.

I think we could refactor kitty preview code so that it saves the tempfiles with different names.

emsquid commented 1 year ago

I was thinking of doing that with caching, we could store tempfiles in some repertory like .cache/pic or something, and not delete them so that we don't need to open the image each time (idk how we could name them to keep them unique and yet remember them), atm the tempfile is immediately being deleted by Kitty because it is not required anymore (Kitty loads the image in its memory, but if it's loaded without id it doesn't remain, there is a flag we can use so that it loads it with a random available id, see here)

emsquid commented 1 year ago

It would be great if we could keep track of image ids, much more efficient, it's so fast once the image is loaded

Maratripa commented 1 year ago

Maybe we remove the option to set IDs manually and set up a hash the image path + unique options (What I mean by this is that I don't know if kitty sets up width and height of the image after loading from memory or it saves it to memory with some width and height) so if I run pic with the same arguments as before, kitty just loads the hashed ID.

emsquid commented 1 year ago

The power of kitty is that it once an image is loaded it can display it anywhere and resize it super quickly, the image is not stored with a fixed memory (kitty is the protocol I developed the most because I was only using this one in the beginning). On the one hand your idea really seems good if we can figure a way to implement it, it could make kitty even better if well done. But on the other hand the problem is the fixed options, with kitty (you can try) it will make it less efficient than loading and then displaying. I already had some thought about that but I didn't want to make some shenanigans implementations... It's up to your imagination as of now, I like your ideas overall, and I will also tell you if something displease me, so try everything you have in mind we are not open source for nothing ;)

Maratripa commented 1 year ago

TODO

Notes

emsquid commented 1 year ago

I'm only wondering how we will get image ids, other than that it seems perfect to me, that's likely the way I would do it, but I couldn't figure a way to remember ids My bad I got it, you want to create an hashmap, this really sounds like a good idea to me, last thing I was thinking about is how long kitty stores images

Maratripa commented 1 year ago

For what I read in the link you sent, if we use the parameter I for kitty, it will reply with a valid i parameter and use that one to store the image. We need a way to get the response from kitty and store that i value as the ID.

emsquid commented 1 year ago

For what I read in the link you sent, if we use the parameter I for kitty, it will reply with a valid i parameter and use that one to store the image. We need a way to get the response from kitty and store that i value as the ID.

Getting that id should be doable, look at the method to check if kitty protocol is supported, it should be approximately the same.

Be careful because I noticed that Kitty and Wezterm don't answer the same way at each request, also to get a response from kitty remove the q parameter

Maratripa commented 1 year ago

To where this is going, I think we should make cache feature in another pr. I'm going to try to get kitty to display multiple files quick ~and dirty~ and then move to another branch, because this feature is almost done. :)

emsquid commented 1 year ago

Let's do this I will merge it when it's done, I should try to implement GIFs with kitty one day too, welp we will clean that later.

Maratripa commented 1 year ago

Sadly, I don't seem to be able to display more than one image at a time with kitty protocol...

emsquid commented 1 year ago

Sadly, I don't seem to be able to display more than one image at a time with kitty protocol...

That's weird, I will look into it tomorrow and give it a try after school!

emsquid commented 1 year ago

@Maratripa did you try to use the I parameter, I tried it quickly and it seems to work fine for me, we doesn't care about the id it returns for now but it's enough to display several images, isn't it ?

Maratripa commented 1 year ago

Yup, you're right... hahahahaha I over complicated things :p