pystardust / ani-cli

A cli tool to browse and play anime
GNU General Public License v3.0
7.9k stars 541 forks source link

feat: thumbnails support #1211

Closed justchokingaround closed 4 months ago

justchokingaround commented 1 year ago

Pull Request Template

Type of change

Description

requires fzf version 0.44.0: https://github.com/junegunn/fzf/releases/tag/0.44.0

didn't document in help, man or readme bc lazy

Checklist

Additional Testcases

justchokingaround commented 1 year ago

i also refactored the args to be in alphabetical order

justchokingaround commented 1 year ago

TODO:

Derisis13 commented 1 year ago

This is a lot of code (50+ lines, which is a 10% increase) already for a feature that's optional. @port19x what do you think?

justchokingaround commented 1 year ago

there will be more if we go for rofi implementation too

port19x commented 1 year ago

This is a lot of code (50+ lines, which is a 10% increase) already for a feature that's optional. @port19x what do you think?

I'm conflicted. 50 loc is a lot indeed, but perhaps warranted for a large feature like thumbnails. The question would then be if we accept a considerable bloating of our codebase for this feature.

Considering this doesn't include the rofi implementation I'd say it's not worth it. iirc jerry has thumbnail support, and that script is >1k loc (likely for other reasons too).

port19x commented 1 year ago

> i also refactored the args to be in alphabetical order

This makes it slightly confusing to review and assess the actualy scope increase caused by the feautre in isolation. Is it possible to extract the reordering into a separate refactor PR?

Not worth the hassle, since it's a naturally self-contained section of code it's not that bad

justchokingaround commented 9 months ago

this is what the implementation looks like. no external dependencies are required, except a minimum version of 0.44.0 for fzf

image image

port19x commented 8 months ago

Please elaborate on the case for image caching

port19x commented 8 months ago

I think I get it. Say I want to watch all the differently named seasons of an anime, or all the one from franchise. Or maybe I want to watch the movies of a large anime. Then I'll search the same common term over and over again, thus there is benefit to caching.

port19x commented 4 months ago

Let's be real, this will never get done and that's not even much of an issue. I'm closing this ancient PR

justchokingaround commented 4 months ago

true lol