feh icon indicating copy to clipboard operation
feh copied to clipboard

use stat for sorting by size like we do with mtime

Open hidroto opened this issue 2 years ago • 2 comments

I think we should change sorting by size to not need a preload of the filelist. there is no need to preload the filelist when we sort by mtime and we can use stat to get the size of the images much like we get the mtime. like so

diff --git a/src/filelist.c b/src/filelist.c                                  
index 8da6e58..51331ce 100644                                                
--- a/src/filelist.c                
+++ b/src/filelist.c   
@@ -466,7 +466,20 @@ int feh_cmp_pixels(void *file1, void *file2)                                              
                                                                                
 int feh_cmp_size(void *file1, void *file2)                                     
 { 
-       return((FEH_FILE(file1)->info->size - FEH_FILE(file2)->info->size));
+       struct stat s1, s2;
+       
+       if (stat(FEH_FILE(file1)->filename, &s1)) {
+               feh_print_stat_error(FEH_FILE(file1)->filename);
+               return(-1);
+       }
+   
+       if (stat(FEH_FILE(file2)->filename, &s2)) {
+               feh_print_stat_error(FEH_FILE(file2)->filename);
+               return(-1);
+       }
+        
+       /* gib_list_sort is not stable, so explicitly return 0 as -1 */
+       return(s1.st_size < s2.st_size ? -1 : 1);
 }       
    
 int feh_cmp_format(void *file1, void *file2)
diff --git a/src/filelist.h b/src/filelist.h
index 7f263dc..b4b8f98 100644
--- a/src/filelist.h
+++ b/src/filelist.h
@@ -71,11 +71,11 @@ enum sort_type {
        SORT_NAME,
        SORT_FILENAME,
        SORT_DIRNAME,
-       SORT_MTIME,
+       SORT_SIZE, 
+       SORT_MTIME, // everything after SORT_MTIME requires a preload of the filelist
        SORT_WIDTH, 
        SORT_HEIGHT,
        SORT_PIXELS,
-       SORT_SIZE, 
        SORT_FORMAT
 };

Is there a reason we are not doing this. sorting by time does not modify the FEH_FILE struct and I think that this should sort the same.

hidroto avatar Feb 15 '23 09:02 hidroto

The reason is "noone thought of it after adding the mtime sort mode in 4fae6007dd64d223cdae3f977003f38e3036bada". So yeah, that's a good idea :) do you want to open a pull request or would you prefer me to just merge the patch you provided?

derf avatar Feb 20 '23 18:02 derf

merging the patch is fine by me. we should also change the man page to say that the preload is not necessary.

hidroto avatar Mar 01 '24 09:03 hidroto