LukeSmithxyz / voidrice

My dotfiles (deployed by LARBS)
GNU General Public License v3.0
4.27k stars 1.21k forks source link

remove 'rotdir' | simplify and improve directory rotation #1341

Open emrakyz opened 1 year ago

emrakyz commented 1 year ago

Those who don't like the complexity of Bash, can use the Dash version. I think this is even better. I didn't think it would have been this easy. This is extremely fast and has very low overhead:

You can still open any file you select and navigate back and forth. It can similarly be adapted to nsxiv but the arguments passed to nsxiv are a little bit different as you can see in the actual file change for this PR. If you need help, I can help about it.

The below is the most optimized version I come up with. fd is a Rust-based, faster, more intuitive find alternative. By using Bash, instead of POSIX sh, you can even remove fd command but it's questionable if It's faster or not. I will probably try benchmarking.

#!/bin/sh
for f in $(fd -F -d "1" -t "f" -e 'jpg' -e 'jpeg' -e 'jxl' -e 'png' -e 'webp' . .); do
    [ "${p}" ] || [ "${f##*/}" = "${1##*/}" ] && p="1"
    [ "${p}" ] && s="${s} ${f}" || b="${b} ${f}"
done
exec imv -f ${s} ${b}

If your filenames are correct, you can instead use the below script. It's a few ms faster without fd and this is almost under 1ms for most directory sizes):

#!/bin/sh
for f in ./*.jpg ./*.jpeg ./*.jxl ./*.png ./*.webp; do
    [ "${p}" ] || [ "${f##*/}" = "${1##*/}" ] && p="1"
    [ "${p}" ] && s="${s} ${f}" || b="${b} ${f}"
done
exec imv -f ${s} ${b}

This method reads image files directly from the current directory $PWD and then uses an array to keep track of all the images. The use of Bash built-in commands like for loop and if condition ensure a faster execution because there is no need to create a subprocess for each image file. The sxiv command is run only once when the selected image is found.

Furthermore, this method takes advantage of array slicing in Bash ("${images[@]:i}" "${images[@]:0:i}"). This feature allows the program to pass the images that come after the selected image (including the selected image) and before the selected image, to sxiv in correct order. This maintains the file order of the directory.

In terms of efficiency, using Bash built-in operations with arrays should consume less system resources compared to lots of pipelines with multiple processes.

The old method uses the rotdir script which, when combined with awk, ls, grep, setsid, and sxiv, creates multiple subprocesses. This increases the overhead for context switching between these processes, leading to more system resource usage and a slower operation than the first method.

Moreover, the use of setsid -f sxiv -aio 2>/dev/null might launch sxiv multiple times, increasing the number of processes and resource usage.

The new method is more readable and maintainable due to its use of simple Bash constructs. The logic is straightforward and doesn't involve external scripts.

On the other hand, the old method requires understanding the rotdir script, making it more complex to read and understand. The maintenance of this script would also need to be considered alongside the lfrc file, adding an extra layer of complexity.

Detailed Explanation

shopt -s nullglob

This line tells bash to treat patterns which don't match any files (globs) as expanding to a null string, rather than themselves. This avoids problems in case there are no listed extensions in the directory.

dir="$0" selected_file="$1"

Here, dir and selected_file are being set to the two arguments that are passed to the bash script at the end ("$PWD" and "$fx"). They are the working directory and the selected file, respectively.

The loop for file in "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}; do will correctly handle filenames with spaces because it does not involve ls. It directly iterates over the filenames matching the glob pattern. Also, [[ -f "$file" ]] ensures that you only add files (not directories) to the images array.

[[ "${images[i]}" = "$selected_file" ]] && { sxiv -abiof "${images[@]:i}" "${images[@]:0:i}" break } done

This is a for loop that iterates over each image in the images array. For each image, it checks if the image equals the selected file. If it does, the sxiv command is executed with all images from the selected one to the end of the list, followed by all images from the start of the list to the selected one. This makes it possible to navigate through the images both forward and backward, preserving the order from lf. After this, the break command is used to exit the for loop because we've found our selected image.

The "$PWD" and "$fx" at the end are the arguments that are passed to the bash script. "$PWD" is the current working directory in lf, and "$fx" is the currently selected file in lf.

${images[@]:i}: This gives you all the elements in the images array starting from index i (which is the index of the selected image file) to the end of the array. In the images array, these are all the image files in the directory that come after (or are the same as) the selected file, in the order they are listed in lf.

${images[@]:0:i}: This gives you all the elements in the images array from the start of the array up to, but not including, index i. In the images array, these are all the image files in the directory that come before the selected file, in the correct order.

2084x commented 1 year ago

when opening an image the terminal goes blank and sxiv doesn't appear until I hit super+q to close lf. file names with white space won't even open, gets exit status 1.

images don't appear before the selected one like you say they do, instead it's the same order as rotdir uses (this would be a fantastic feature if it worked).

are these things happening to you?

and why would you want to start in fullscreen and remove the bar by default?

emrakyz commented 1 year ago

@2084x

Hi! Thanks for trying.

It works fine for me but I use "imv" instead of "sxiv" since I am on Wayland. Maybe that's the case?

I can look for a fix.

why would you want to start in fullscreen and remove the bar by default?

I only want to see the image using an image viewer in order to get rid of the distractions. I probably didn't realize the default is different on Luke's config.

emrakyz commented 1 year ago

@2084x

Hi again! I have fixed it and improved the efficiency even more removing two additional programs. Can you please try it and confirm?

It's extremely fast and efficient for me and works for any kind of white spaces including tabs and new lines.

The loop for file in "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}; do will correctly handle filenames with spaces because it does not involve ls. It directly iterates over the filenames matching the glob pattern. Also, [[ -f "$file" ]] ensures that you only add files (not directories) to the images array.

With this script, we don't use any program to process anything inside lf. We directly open the images removing the use of awk, grep, ls, sort, setsid and repeated instances of sxiv. The only program we use is Bash itself.

2084x commented 1 year ago

Files with white space names now open, but the rest of the issues are still present. This repo uses sxiv and X so you should be writing the function specifically for those.

emrakyz commented 1 year ago

@2084x

Thanks for trying.

I think I fixed it with the latest change now. Can you try it again?

sxiv and imv are very similar in nature, both of them are lightweigh image viewers but I didn't know the related difference that causes the issue:

The main difference in this context is that imv takes a list of image files as command-line arguments and starts displaying the image specified by the first argument, whereas sxiv starts displaying the first image in the list.

In the original script, we split the array into two parts around the selected image, and then concatenate the two parts in reverse order. This results in a new array where the selected image is the first item, and imv starts with this image.

However, sxiv doesn't work this way - it will always start with the first image in the list, regardless of the order you specify. Therefore, we need to modify the array of images directly, before calling sxiv.

This would fix the issue.

emrakyz commented 1 year ago

@2084x

Here is a very short sample video.

You can see that I am opening the third picture and returning from the third picture to the second picture.

output.webm

2084x commented 1 year ago

Can you try it again?

Makes no difference for me. Perhaps we need someone else to test to make sure this isn't just something going wrong on my end.

Here is a very short sample video.

Yea cool it works for you but as I said, you need to test this with sxiv / X / lukes configs....

emrakyz commented 1 year ago

@2084x

Hi! This time I tried another fix. If you wish to try one more time, you are free to do so :)

The original imv command in my script had images from the current selection onward followed by images before the current selection, essentially rotating the list to start at the selected image. This is necessary because imv always starts at the first image given on the command line.

sxiv, on the other hand, as I've just learned, accepts a -n or --start-at option to specify which image to start at, which simplifies the command. This means you don't need to reorder the arguments, and can go both forward and backward from the starting image.

This version of the script now calls sxiv -n "$((i + 1))" "${images[@]}". The "$((i + 1))" is used because array indices in Bash start from 0, but sxiv starts counting images from 1. The ${images[@]} passes all images as arguments to sxiv. When -n is used, sxiv will start at the image at the passed index.

2084x commented 1 year ago

Ok I've realised what's going on... you just needed to remove the -i flag now it opens correctly. the command should be setsid -f sxiv -aon "$((i + 1))" "${images[@]}"

There's still some issues:

  1. The images do display before and after the one you select, but the order is still wrong. They are sorted by extension, then alphabetically. It should just be alphabetical / natural order, as is shown by default in lf. or perhaps there's a way to read the sort method lf is currently using?

  2. the -o flag is used to print to stdout so that any images you mark by hitting 'm' are selected when you close sxiv and return to lf. when I mark images nothing is selected. I believe this was originally the purpose of

    lf -remote "send select \"$file\""
    lf -remote "send toggle"
emrakyz commented 1 year ago

Why would you use setsid?

They are sorted in a lexicographic order. It's pretty much very close to the default sorting. We can add much more configured sorting but it would add a complexity to the script.

Lexicographic order is alphabetic in nature, but it has a different way of handling numbers if the file has them: 10 would come after 1 instead of 2. You can make a workaround by naming your files, 01, 02, 03 and so on.

The biggest hit of this script is using Bash only and bash built-in functions, so we never use another program except the image viewer itself and we don't open multiple subprocesses. That's the efficiency of this script. Lf file order mechanism is much different. Of course everything is doable but it's above the purpose. In my use case, I don't encounter -what I call- a weird ordering of images. It's mostly alphabetic.

2084x commented 1 year ago

Why would you use setsid?

I'm just saying you should be consistent with what luke was already using. He doesn't want the window swallowed. This isn't your personal repo.

They are sorted in a lexicographic order.

Lexicographic is fine, but the way they appeared sorted for me was like this:

a.jpg b.jpg c.jpg a.png b.png c.png a.gif b.gif etc...

This is sorting by extension.

sxiv -na

this doesn't work. n has to be the last flag, just as I wrote it out.

emrakyz commented 1 year ago

@2084x

This isn't your personal repo.

Oh, of course I know that. All I want is to improve the already intended method, not to do a completely different thing. I guess he wanted to be able to use the image viewer after the terminal is closed. I wasn't sure about this. So I can add it.

Thanks by the way for thorough testing. I'll think about the sorting, I'll try some cases.

In my opinion that much correct ordering of files when I am rotating is not important so I won't use it, but it can be added for this repo.

We can add ls and sort -V or we can create an image array with printf or echo. I'll probably go with ls and sort -V:

I am pretty confident with the recent change that you will like it and all of the mentioned problems will disappear.

Here is the output from my trial:

Opening the first file and going to right: /home/emre/pics/a.jpeg /home/emre/pics/a.png /home/emre/pics/a.webp /home/emre/pics/b.jpg /home/emre/pics/b.png //home/emre/pics/b.webp /home/emre/pics/c.jpg /home/emre/pics/d.jpg

Opening the last file and going to the left: /home/username/pics/d.jpg /home/username/pics/c.jpg //home/username/pics/b.webp /home/username/pics/b.png /home/username/pics/b.jpg /home/username/pics/a.webp /home/username/pics/a.png /home/username/pics/a.jpeg

What do you think now?

We added the -n mode for sxiv in order to handle input similar to imv. We added setsid in order to keep the image after closing the terminal. We added version sorting as seen inside lf.

The problem for "sxiv" that it doesn't work as "imv". With imv, you can browse files by opening a singular file with created image arrays. Sxiv can't do that. The advantage of the first version is that it only passes the necessary files to imv, while this version passes all files. But it's still efficient enough, it's just a comparison.

Hope you like it!

My Method: Maybe some people prefer the other way where we don't use any separate programs and only handle a single file (it needs imv):

    image/*) bash -c '
            shopt -s nullglob
            dir="$0"
            selected_file="$1"
            images=()
            for file in "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}; do
                [[ -f "$file" ]] && images+=("$file")
            done
        for ((i=0; i<${#images[@]}; i++)); do
                [[ "${images[i]}" = "$selected_file" ]] && {
                    imv -f "${images[@]:i}" "${images[@]:0:i}"
                    break
                }
            done
        ' "$PWD" "$fx";;

Newest Method for this Repo:

        image/*) bash -c '
            shopt -s nullglob
            dir="$0"
            selected_file="$1"
            images=()
            for file in "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}; do
                [[ -f "$file" ]] && images+=("$file")
            done
            images=($(printf "%s\n" "${images[@]}" | sort -V))
            for ((i=0; i<${#images[@]}; i++)); do
                [[ "${images[i]}" = "$selected_file" ]] && {
                    setsid sxiv -aon "$((i + 1))" "${images[@]}"
                    break
                }
            done
        ' "$PWD" "$fx";;

Regarding how the image viewers handle the images, 'imv' may require less memory initially since it opens the selected image first and then the rest. 'sxiv', on the other hand, is called with all the images at once, which may require more memory if there are a lot of large image files. However, the difference would be minimal and probably unnoticeable.

2084x commented 1 year ago

It still doesn't work for me. When you sort the images any that have spaces are split up into multiple strings and they won't be loaded in sxiv. I don't know how to avoid that. It seems like with your script it's either you get all the files, but they're out of order OR you get the files in order, but you're missing any with spaces in the name.

I had a go at writing the function myself and came up with this:

image/*) find "$PWD" -maxdepth 1 -type f -regex ".*\.\(png\|jpg\|jpeg\|gif\|webp\|avif\|jxl\|tif\|ico\|bmp\|raw\)\(_large\)*$" | sort -fV |
    setsid -f sxiv -aion \
    $(find "$PWD" -maxdepth 1 -type f -regex ".*\.\(png\|jpg\|jpeg\|gif\|webp\|avif\|jxl\|tif\|ico\|bmp\|raw\)\(_large\)*$" | sort -fV |
    while read -r file; do
        echo "$file"
        [ "$file" = "$f" ] && break
        done | wc -l) |
    while read -r file; do
        [ -z "file" ] && continue
        lf -remote "send $id select \"$file\""
        lf -remote "send $id toggle"
    done &
    ;;

It's a bit ugly because you have to find the file list twice, but this way everything is in order and there are no files missing. You also don't have to use bash, arrays or external scripts. Maybe someone can improve this. I'm sure there's a smarter way to get the value for -n than what I've done here.

emrakyz commented 1 year ago

@2084x

I overlooked it since I don't use sorting myself.

Thanks to your thoroughly trying, I fixed it again:

The problem was that the script was creating a new array, images, from a string that has each file name separated by a newline. However, if a file name contains spaces, the name is split at each space.

We can fix this using the Internal Field Separator. By temporarily setting IFS to a newline character, we can ensure that array elements are split on newlines instead of any whitespace:

image/*) bash -c '
            shopt -s nullglob
            dir="$0"
            selected_file="$1"
            images=()
            for file in "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}; do
                [[ -f "$file" ]] && images+=("$file")
            done
            sorted_images=()
            while IFS= read -r line; do
                sorted_images+=("$line")
            done < <(printf "%s\n" "${images[@]}" | sort -V)
            for ((i=0; i<${#sorted_images[@]}; i++)); do
                [[ "${sorted_images[i]}" = "$selected_file" ]] && {
                    setsid sxiv -aon "$((i + 1))" "${sorted_images[@]}"
                    break
                }
            done
        ' "$PWD" "$fx";;

You also came with a solid approach but your method can be inferior:

Using Bash arrays (images and sorted_images) to store the image filenames, while the second one relies on pipes is preferable. Using arrays can be more efficient because it avoids the overhead of creating new processes for each pipe. It also allows for greater flexibility in manipulating the data since you can easily add, remove, or reorder elements within the array.

Bash method only uses a single seraching operation to gather all the image files. In contrast, the second one has to call find twice: once to pipe into sxiv and another to feed the while loop. This could lead to slightly worse performance.

The setsid command is used in both methods, but in the bash method, it's explicitly tied to the sxiv command, which launches it. This makes it clearer that a new session is being created for the image viewer. In the second method, setsid is used with a pipe, which is less clear.

Although the bash method may look more complex due to the extensive use of bash constructs, it is actually more compact than the other achieving the similar thing.

In the bash method, the script breaks out of the loop as soon as the selected image is found. This ensures that the script doesn't do unnecessary work. In contrast, the second method will continue to process even after the selected image is found, potentially leading to more use of computational power.

2084x commented 1 year ago

Ok awesome sorting is working now! still some things to consider:

  1. sort -fV is closer to the way lf displays files naturally.
  2. if any files are selected in lf I get exit status 1 when trying to open sxiv. you need to use $f instead of $fx.
  3. you need to collect the stdout and select any marked images in lf when sxiv quits like this or maybe you can think of a better way.
    | while read -r file; do
    [ -z "$file" ] && continue
    lf -remote "send $id select \"$file\""
    lf -remote "send $id toggle"
    done &
emrakyz commented 1 year ago

@2084x

Ok awesome sorting is working now! still some things to consider:

Nice to hear it works.

  1. I added -fV option for sort.
  2. I added an ability to browse selected files and unselect them automatically at the end.
  3. I minimized the script in general.

Please tell me if you think about a possibly wanted or good feature or any problem in general.

IFS=":" read -r -a selected_files <<< "$2" This line is added to the updated script to handle multiple file selections. When multiple files are selected in lf, this code will split the file list into an array named selected_files.

*files=("${selected_files[@]}" "$dir"/.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG})* This line is added to handle the case where multiple files are selected in lf. It generates an array files which includes both the selected files ("${selected_files[@]}") and all the image files in the current directory ("$dir"/.

lf -remote "send $3 unselect" This line is added in the updated version, which deselects all files in lf after the image viewer is opened.

' "$PWD" "$fx" "$fs" "$id";; In the updated version, there are more arguments passed to the bash script to handle additional functionality. "$PWD" is the current directory, "$fx" is the currently focused file, "$fs" is the list of selected files, and "$id" is the client id of the lf instance.

2084x commented 1 year ago

I think you're confused by my previous comment. Whatever you've done now has broken things again.

emrakyz commented 1 year ago

@2084x

I have returned the most recent working script for now.

If I can understand this part of the Luke's current script, I can add the functionality:

    [ -z "$file" ] && continue
            lf -remote "send select \"$file\""
            lf -remote "send toggle"

This part is run after the sxiv command. How does it work for selected files if sxiv is already opened?

eylles commented 1 year ago

since luke finally decided to drop sxiv for a maintained image viewer y'all may want to take a look at some of the extras scripts developed for nsxiv as they provide more functionality and with the likes of nsxiv-env it may even be possible to add support for using other image viewers like imv and feh without too many changes.

link description
nsxiv-rifle open all images in directory starting at pased image
nsxiv-open tries to open files on nsxiv, will create tmp images of files or open them with xdg-open when all else fails
nsxiv-env default flags for nsxiv via env var
emrakyz commented 1 year ago

@eylles

nsxiv-rifle achieves the similar function but I still think my method is closer to Luke's way of handling things.

It only uses one shell and built-in bash functions without relying on find, grep etc. and is more minimal.

More features can be added if needed.

2084x commented 1 year ago

the way the rifle script gets the file list gave me some ideas. we can alter that command a little and get a different list based on $lf_sortby which is how the files are currently displayed in lf. I added a line for reversing the list as well. probably excessive for this repo but I wanted to share anyway.

image/*) lf -remote "send $id openimg" ;;

-----

cmd openimg ${{
    isimg() {
        grep -iE '\.(jpe?g|png|gif|svg|webp|tiff?|heif|avif|ico|bmp|jxl|raw)$'
    }
    listfiles() {
        images=($(find -L "$PWD" -maxdepth 1 -type f -exec stat -c "$1 %n" {} + | isimg | sort -n | cut -d' ' -f 2-))
    }
    case $lf_sortby in
        natural) images=($(find -L "$PWD" -maxdepth 1 -type f | isimg | sort -fV)) ;;
        ext) images=($(find -L "$PWD" -maxdepth 1 -type f -exec ls -X {} + | isimg)) ;;
        size) listfiles %s ;;
        time) listfiles %W ;;
        atime) listfiles %X ;;
        ctime) listfiles %Z ;;
    esac
    [ $lf_reverse = "true" ] && readarray -td '' images < <(((${#images[@]})) && printf '%s\0' "${images[@]}" | tac -s '')
    for ((i = 0; i < ${#images[@]}; i++)); do
        [ "${images[i]}" = "$f" ] && {
            setsid -f nsxiv -aon "$((i + 1))" "${images[@]}" | while read -r file; do
                [ -z "$file" ] && continue
                lf -remote "send $id toggle \"$file\""
            done &
            break
        }
    done
}}
eylles commented 1 year ago

well, if you like that one there's a personal script i use for nsxiv with vifm, however this makes use of a pr that got merged and is scheduled to land on nsxiv v32.

#!/bin/sh

export NSXIV_OPTS="-aqo"

selected_file=$(nsxiv-rifle "$@")

if [ -n "$selected_file" ] && [ -e "$selected_file" ]; then
  vifm --server-name "$VIFM_SERVER_NAME" --remote +"goto \"$selected_file\""
fi
exit

worth mentioning the nsxiv-rifle on my system uses nsxiv-env instead of nsxiv.

what all that does is that whenever i am viewing an image on nsxiv and quit with Q nsxiv will output the file name and this script will tell vifm to go to said file.

emrakyz commented 10 months ago

@2084x @eylles @marcoacarvalho @jlaw

You can still open any file you select and navigate back and forth. This is the most optimized version.

#!/bin/sh
for f in ./*.jpg ./*.jpeg ./*.jxl ./*.png ./*.webp; do
    [ "${passed}" ] || [ "${f##*/}" = "${1##*/}" ] && passed="1"
    [ "${passed}" ] && sa="${sa} ${f}" || b="${b} ${f}"
done
exec imv -f ${sa} ${b}