-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove 'rotdir' | simplify and improve directory rotation #1341
base: master
Are you sure you want to change the base?
Conversation
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. The use of sort -V (version sort) also contributes to maintaining the file order as it sorts version numbers within text. 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. In terms of file ordering, the rotdir script doesn't have explicit sorting. This might not necessarily match the desired order in some cases. Lastly, the second method uses lf -remote to interact with the lf file manager. The command is run each time a new image is selected, adding an additional level of complexity and overhead to the process. The first 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 second 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. We can even get rid of ls and sort by using bash globbing alone but then the files are ordered lexicographically, not numerically. Meaning they would go to 10th after 1st rather than 2nd. There could be a workaround though with naming the files with leading zeroes such as 01, 02 and so on. # 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 image files 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. images=($(ls "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl} | sort -V)) The ls command lists all the files in the directory that have certain image extensions. This list of files is piped (|) to sort -V, which sorts the list in version number order (which is similar to natural order for filenames). This sorted list of images is stored in 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.
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? |
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.
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. |
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 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. |
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. |
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. |
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 |
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.
Yea cool it works for you but as I said, you need to test this with sxiv / X / lukes configs.... |
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. |
Ok I've realised what's going on... you just needed to remove the -i flag now it opens correctly. the command should be There's still some issues:
|
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. |
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.
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.
this doesn't work. n has to be the last flag, just as I wrote it out. |
Oh, of course I know that. All I want is to improve the already intended method, not to do a completely different thing. 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: Opening the last file and going to the left: What do you think now? We added the -n mode for sxiv in order to handle input similar to imv. 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):
Newest Method for this Repo:
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. |
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:
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. |
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:
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. |
Ok awesome sorting is working now! still some things to consider:
|
Nice to hear it works.
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" files=("${selected_files[@]}" "$dir"/*.{jpg,jpeg,png,webp,bmp,tiff,tif,raw,ico,exif,heic,heif,gif,avif,jxl,JPG,PNG}) lf -remote "send $3 unselect" ' "$PWD" "$fx" "$fs" "$id";; |
I think you're confused by my previous comment. Whatever you've done now has broken things again. |
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:
This part is run after the sxiv command. How does it work for selected files if sxiv is already opened? |
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.
|
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. |
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.
|
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 |
@2084x @eylles @marcoacarvalho @jlaw You can still open any file you select and navigate back and forth. This is the most optimized version.
|
@emrakyz I suggest making an additional pull request with the dash version. What you've got here is really cool. Just note that files with special characters and spaces will cause this to behave unexpectedly. For example, you might select a file with spaces in its name, but the next one will be opened instead. This problem does not exist in the 'rotdir' version. I can help with the Since this version has no bashisms, maybe using For reference, this is what I added to my
Thank your for publicizing your bash and dash versions of this! |
Now I use the Zsh version that is even faster. We are not required to use a loop and checks with logical controls.
|
@emrakyz Wow! Not only is this more efficient, but it solves the problem with special filenames. I added this as a one-liner to
Thanks for sharing! |
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 intuitivefind
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.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):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.