feat: add thumbnail generation toggle

[ZwodahS]

Summary

#930 is a blocker for me because I mainly use it for sound at the moment.

Haven't wrote python for a while and isn't really familiar with the current convention for python so let me know if anything need fixing.

Tasks Completed

• Platforms Tested:
  • Windows x86
  • Windows ARM
  • macOS x86
  • macOS ARM
  • Linux x86
  • Linux ARM
• Tested For:
  • Basic functionality
  • PyInstaller executable

[CyanVoxel]

So there's an interesting quirk of the functionality here that I'd like to explain before going on with the rest of the review:

First, I've been meaning to add an option to toggle off thumbnails for quite some time. Non-cached thumbnails (mostly audio files with waveform generation) can take a non-trivial amount of time to generate, as you're already familiar with. #930 is a step in that direction, but I feel that it would be even more appropriate to have the ability to toggle off all thumbnail generation rather than audio waveforms specifically (at least for now, being the first setting of its kind in here).

That being said, here's the quirk of the code here: The setting check isn't surrounding the audio waveform generation step, but rather the entire non-cached thumbnail generation step. Meaning that only the (fast) cached thumbnails are shown while no new thumbnails are generated and default icons are used for everything else. So in a roundabout way, this is actually the behavior I prefer over the original intention of #930.

For the rest of this review I'll be referring to this viewpoint and making suggested name changes for various variables and translations.

---

Outside of implementing any review feedback and fixing up the Ruff and MyPy errors, this should be pretty good to go if you'd like to continue in this direction 👍

[ZwodahS]

got it. I think I misunderstood that part of the code, assuming that it is meant only for sound (because of the comment block above it).

Now that I read it again, I realised it is a handle for all filetypes.

Do you want me to do the changes on github, or rebase/squash it or add additional commits ?

[CyanVoxel]

Do you want me to do the changes on github, or rebase/squash it or add additional commits ?

Additional commits would be best, as all the commits here will eventually get squashed into a single one when merging into main.

And be careful if you decide to commit the review suggestions directly from GitHub as they don't change all instances of the variable names. Personally I feel it's easier to just make all the changes in your editor.

[ZwodahS]

Sounds good, that's what I prefer too.

[ZwodahS]

I also take the liberty to shift the inner if not image to the outside if block since that makes more sense.

edit: it looks like the github diff made it look like there are a lot of changes.

[CyanVoxel]

Everything looks great here! Thank you so much for your work on this!

Edit: Still some minor formatting that needs to be done with Ruff, I can take care of this too for you if you'd like:

[ZwodahS]

Let me do it since I might add a few more features later on and it might be good to go through the full workflow once.

[ZwodahS]

Do you want me to rebase it to the new main to fix the conflict ?

[CyanVoxel]

Let me do it since I might add a few more features later on and it might be good to go through the full workflow once.

Sounds good, future PRs would be greatly appreciated!

Do you want me to rebase it to the new main to fix the conflict ?

You can go ahead and rebase or do a merge commit from main into this one, or if you could manually fix the conflict by adding the or filepath.is_dir() portion that was added to main in the meantime - whichever is easier for you. Just note that in the future it's discouraged to rebase/force push during a PR review since it makes reviewing more difficult, but you're okay to do it this time since this one is almost done and I'm aware that you might do it.

[ZwodahS]

Agree, I generally dislike force pushing during PR because of how messy it might become, but I feel like the diff might be clearer if it is a diff against the new main rather than the old main.

[CyanVoxel]

Thank you for your work on this!