jarun / nnn

n³ The unorthodox terminal file manager
BSD 2-Clause "Simplified" License
19.1k stars 760 forks source link

preview-tabbed: show sxiv/nsxiv in thumbnail mode when watching Pictures folder #1834

Closed TheUtopian closed 5 months ago

TheUtopian commented 7 months ago

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.

jarun commented 7 months ago

Why not post it in the discussions thread for others to use? I don't see a massive value addition. Not everyone stores pictures in a specific directory.

TheUtopian commented 7 months ago

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.

KlzXS commented 6 months ago

@TheUtopian Do you plan on continuing to work on this?

TheUtopian commented 6 months ago

@KlzXS, Yes, I think I found a solution, it's not as elegant as I initially wanted, but it seems to work pretty good. I'll update the PR. I'd be glad to receive any advice on improving the code

KlzXS commented 6 months ago

@TheUtopian please address the issues we raised.

TheUtopian commented 6 months ago

@KlzXS, @N-R-K, thanks for your comments.

I'm fairly sure you can do something like { command1 || command2 } &.

If you'll take a look at the last commit you'll see I was using something similar: ( nsxiv -te "$XID" "$FILE" 2>/dev/null || $TERMINAL "$XID" -e nnn "$FILE" ), but that didn't work, it was opening (n)sxiv tabs and not closing them. I've tried to use curly brackets as well, but it was just returning an error.

But I think I found a better solution: I've noticed that if we use if statement that always run, it will open (n)sxiv and if (n)sxiv will abort, script will run the else statement with nnn. So we can drop if statement with find... entirely:

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?

Why is there a glob before as well? If the picture dir is "/picture" why should "/some/random/dir/picture" match it?

You're right, I'll remove the left match, I was also considering regex but as I read, the glob match should be faster.

N-R-K commented 6 months ago

and if (n)sxiv will abort, script will run the else statement with nnn

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?

TheUtopian commented 6 months ago

If the above fails for example, then it's not going to run anything else. Or am I missing something?

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
} &

This would "work" but consider what I said about (n)sxiv exiting with non-zero for other reasons too, what happens in those cases?

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.

KlzXS commented 6 months ago

If you'll take a look at the last commit you'll see I was using something similar

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.

what happens in those cases?

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.

TheUtopian commented 5 months ago

@KlzXS, I have tried to do what N-R-K suggested, but unfortunately it also creates a subshell, so the tabs don't close. I found another solution which is much faster, I took every image extension supported by imlib2 which is used by sxiv and nsixv. The only disadvantage I see is that it looks ugly.

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
N-R-K commented 5 months ago

I took every image extension supported by imlib2

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.

TheUtopian commented 5 months ago

@N-R-K, I squashed everything into one commit

jarun commented 5 months ago

Thank you!