jun7 / rox-filer

ROX file manager
24 stars 6 forks source link

thumbnail_program(): return path only if executable #230

Closed JakeSFR closed 2 years ago

JakeSFR commented 2 years ago

Hey Jun,

There's a minor problem with external thumbnailers. To reproduce:

  1. Create a new thumbnailer script, e.g. for PDFs, named application_pdf in MIME-thumb directory:
#!/bin/sh
# create thumbnail for PDF
# $1-path to source, $2-path to output, $3-pixel size 

exec pdftocairo -singlefile -scale-to "$3" "$1" -png "${2%.*}"
  1. Don't make it executable, or make it non-executable, if it already is,
  2. Make sure to do "Thumbnails -> Purge thumbnails disk cache",
  3. Enter a directory that contains some PDFs and enable Thumbnails,
  4. Nothing happens (seemingly), which is expected,
  5. Now, make that script executable, re-enter the above direcory and enable Thubmnails,
  6. Nothing happens, no thumbnails are being created, which is unexpected.

The problem seems to be that even when the thumbnailer program/script is not executable, ROX still calls it (not generating any thumbnails, though), but what's worse, ROX remembers to not create thumbnails the next time you enter that directory (unless you restart ROX).

Not sure if there's a better way, but the fix that works for me is to simply check if the _thumbnailprogram is actually executable and only then return its path.

jun7 commented 2 years ago

BTW Does middle click the scan button in the toolbar work?

JakeSFR commented 2 years ago

BTW Does middle click the scan button in the toolbar work?

Oh, actually, it does. I only tried deleting the cache via "Purge thumbnails disk cache" option, wasn't even aware of the middle-click action. But still, now at least the non-executable programs won't be called unnecessarily.

On the other hand, I just realized that the thumbnailer programs can be also AppDirs, in which case the G_FILE_TEST_IS_EXECUTABLE test returns the AppDir's directory exec bit, not the AppRun's inside. Maybe it's not a big deal, since dirs usually have the exec bit(s) set, so AppDir thumbnailers should still work as before. But if you think it could pose some problems, feel free to revert the merge, I'm happy with the middle-click solution as well.

Thanks!

jun7 commented 2 years ago

Thank you!