-
-
Notifications
You must be signed in to change notification settings - Fork 769
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
preview-tabbed: show sxiv/nsxiv in thumbnail mode when watching Pictures folder #1834
Conversation
Why not post it in the discussions thread for others to use? |
I think it's very convenient to show thumbnails in Pictures instead of just folders and I think this feature will only improve experience for most users. Users who save images in different folders also will be able to easily modify the code to include their favorite folders. For others who don't use sxiv or nsxiv it'll act as before and if someone wouldn't like this behavior they could just comment out 3 lines, so I don't see any downside to not enabling this by default. P.S. I found a bug in this PR so I will change it to draft for now. |
@TheUtopian Do you plan on continuing to work on this? |
@KlzXS, Yes, I think I found a solution, it's not as elegant as I initially wanted, but it seems to work pretty good. |
plugins/preview-tabbed
Outdated
@@ -177,7 +180,21 @@ previewer_loop () { | |||
fi | |||
;; | |||
inode/directory) | |||
$TERMINAL "$XID" -e nnn "$FILE" & | |||
if [[ -n $PICTURES_DIR && "$FILE" == *"$PICTURES_DIR"* ]] ; then | |||
if find "$FILE" -maxdepth 1 | file -bLf- --mime-type | grep -q image ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are some pretty expensive operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this correctly:
IF we are currently hovering over the "Pictures" directory AND it directly contains at least one file whose MIME is an image THEN we preview the directory in a specified program if available ELSE we do what we were doing previously.
What do sxiv
and nsxiv
do if you open a directory that contains no images? Is it any different to what happens if it contains an image? I can't try this out at the moment. If it's not a significant difference just drop this check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do sxiv and nsxiv do if you open a directory that contains no images?
It exits due to not having any images to show.
Is it any different to what happens if it contains an image?
If it contains images then those images will be opened in thumbnail view (since -t
flag is specified).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying that .
I assume it exits with a non-zero error code. Just trying it and falling back if it fails should be less expensive, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it exits with a non-zero error code
It exits with 1
, yes.
Just trying it and falling back if it fails should be less expensive
Yeah that's probably better. But how would you do that? The (n)sxiv process gets backgrounded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a slight inconvenience.
I'm fairly sure you can do something like { command1 || command2 } &
.
A bit uglier than I hoped for the final result, but it should do the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit uglier than I hoped for the final result, but it should do the trick.
Well, nsxiv
might exit with non-zero for other reasons too. So IDK about that.
Also don't we send signals to close/stop the backgrounded process? That's almost certainly going to result in a non-zero exit code as well and trigger command2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, nsxiv might exit with non-zero for other reasons too
If it fails for any reason I think it's still better to fall back. A preview > no preview.
Also don't we send signals to close/stop the backgrounded process?
To be honest, I was away when the whole preview thing was being implemented and never got into it, so I have no idea how it works.
Would the signal be sent to nsxiv
or the subshell? If it's the subshell I think we're fine. It makes more sense that it should be the subshell. How would we even know the PID of a process it spawns?
@TheUtopian please address the issues we raised. |
@KlzXS, @N-R-K, thanks for your comments.
If you'll take a look at the last commit you'll see I was using something similar: But I think I found a better solution: if type sxiv >/dev/null 2>&1 ; then
sxiv -te "$XID" "$FILE" &
elif type nsxiv >/dev/null 2>&1 ; then
nsxiv -te "$XID" "$FILE" &
else
$TERMINAL "$XID" -e nnn "$FILE" &
fi What do you think?
You're right, I'll remove the left match, I was also considering regex but as I read, the glob match should be faster. |
But that's not what the code is doing, though. sxiv -te "$XID" "$FILE" & If the above fails for example, then it's not going to run anything else. Or am I missing something? I'm assuming you meant to do something like this: if sxiv -te "$XID" "$FILE" 2>/dev/null; then
: # no-op
elif nsxiv -te "$XID" "$FILE" 2>/dev/null; then
: # no-op
else
$TERMINAL "$XID" -e nnn "$FILE"
fi & This would "work" but consider what I said about (n)sxiv exiting with non-zero for other reasons too, what happens in those cases? |
You're totally right, It doesn't work that way unfortunately, I was a little hasty. I've also tired this but it still opens new tabs without closing them: {
nsxiv -te "$XID" "$FILE"
if [ $? -ne 0 ]; then
$TERMINAL "$XID" -e nnn "$FILE"
fi
} &
If I understood correctly, even if nsxiv returns non-zero value e.g. when we close it by switching the preview file, nnn will be opened and closed immediately, so it shouldn't be noticable. |
I missed that. I've since tried my suggestion on an actual machine and confirmed that you indeed can't terminate the subshell directly. Try what @N-R-K suggested.
Do we actually care about differentiating those? As @TheUtopian said, at worst we just spawn another preview process and it fails if, for example, it doesn't have permission. |
@KlzXS, I have tried to do what N-R-K suggested, but unfortunately it also creates a subshell, so the tabs don't close. if \ls -f "$FILE" | grep -qm 1 "jpg$\|jpeg$\|png$\|svg$\|gif$\|jxl$\|webp$\|ico$\|bmp$\|ani$\|argb$\|ff$\|heif$\|heifs$\|heic$\|heics$\|avci$\|avcs$\|avif$\|avifs$\|jfif$\|jfi$\|jp2$\|j2k$\|iff$\|ilbm$\|lbm$\|ppm$\|pgm$\|pbm$\|qoi$\|ps$\|raw$\|arw$\|cr2$\|dcr$\|dng$\|nef$\|orf$\|raf$\|rw2$\|rwl$\|srw$\|tga$\|tiff$\|xbm$\|xpm$"; then
if type sxiv >/dev/null 2>&1 ; then
sxiv -te "$XID" "$FILE" &
elif type nsxiv >/dev/null 2>&1 ; then
nsxiv -te "$XID" "$FILE" &
else
$TERMINAL "$XID" -e nnn "$FILE" &
fi
else
$TERMINAL "$XID" -e nnn "$FILE" &
fi |
Imlib2 allows for disabling many of the loader support. On my system many of those formats are not supported because I've disabled them. Just assume that if the user has picture directory enabled, that it contains pictures. |
@N-R-K, I squashed everything into one commit |
Thank you! |
It'll use
xdg-user-dir PICTURES
to get Pictures folder location and if user enters the folder it'll use sxiv/nsxiv to show thumbnails instead of nnn.