ckardaris / ucollage

An extensible command line image viewer inspired by vim
GNU General Public License v3.0
210 stars 3 forks source link

Support other type of image extensions #2

Closed KevCui closed 3 years ago

KevCui commented 3 years ago

In this PR, I use file command to check whether a file type contains image data or bitmap, if yes, then it's an image file. This change will make ucollage support other type of image extensions.

ckardaris commented 3 years ago

One of my initial implementations had a functionality like the one you propose. The thing is that checking the filetype for a huge number of files has a huge overhead in start up time. I think we have to find a solution that is faster in that regard.

KevCui commented 3 years ago

In the latest commit, I removed grep pipe in order to increase speed.

I tested script in a folder with 720 images, to see the first images loaded:

It looks like it's still not fast enough.

ckardaris commented 3 years ago

It's really interesting. Thanks a ton for working on that. I believe we can make it faster with some more research. If not we are merging that soon. I currently am working on some other functionalities and code changes, so you can work on a solution as long as you like.

ckardaris commented 3 years ago

I think I found a solution that is probably better. I am testing it now. I am fetching only the necessary images to show in the first screen. And then while the program loops waiting for input, I do the rest of the checking. I will the push the changes in a testing branch. I will let you know.

KevCui commented 3 years ago

In the latest commit, I used a predefined extension list, if the file extension is not in the list then it will check its file type. If the check result is an image, then the new extension will added to the list for following detection.

I tested script in a folder with 720 images, to see the first images loaded:

However, there is one case that the process speed may not be ideal: when a folder contains large number of non-image files. Well, in this case, I'd suggest someone run the script with file names as argument like: ucollage *.jpg

ckardaris commented 3 years ago

Check out commit 3a83474de6a9c0a2d4a5d8f872097dbe4728ccbb in testing branch. I believe it is quite good. The problem with scanning files which are not images still remains. Because even with my implementation if it can't find even some initial images the program loops until it consumes all the files.

Edit: I have added outputting dots when the file scanned is not an image, so the user has an indication that something is off

ckardaris commented 3 years ago

@KevCui I merged my implementation to master. So, unless you have some other suggestion regarding this topic, I will be closing this PR. I hope with your help we can further improve this script in other areas as well.

KevCui commented 3 years ago

@ckardaris I checked testing branch. It looks awesome :+1: