Add an option to show image from editor in folder

[user1823]

This feature is quite handy, especially when you want to see an enlarged view of the image.

For e.g., see https://forums.ankiweb.net/t/copying-images-out-of-cards-is-hard/20272/6

Credits: @abdnh's Reveal in File Manager add-on (https://github.com/abdnh/anki-misc/tree/master/reveal_in_file_manager)

[abdnh]

It would be nice to open the file manager with the file selected. Here are relevant examples from add-ons:

https://github.com/ijgnd/anki21__OpenInExternalEditor_Rename_Duplicate_for_Image_Audio_Video/blob/master/src/showInFilemanager.py

https://github.com/abdnh/anki-misc/blob/master/reveal_in_file_manager/__init__.py

[user1823]

It would be nice to open the file manager with the file selected.

What is the use case?

[abdnh]

It potentially allows for more operations on the file (e.g. copy, opening in some editor, etc) but probably just opening the image is easier to implement, as you don't need to handle different file managers on Linux AFAIK.

[user1823]

It potentially allows for more operations on the file (e.g. copy, opening in some editor, etc)

Ability to copy the image has already been added in #3362

Adding the ability to open the image in external editors doesn't seem important enough to justify the complexity. Plus, if we open the file manager instead of the file itself, opening the image will require an additional click.

[glutanimate]

This looks like code extracted almost 1:1 from Image Occlusion Enhanced, including the lack of type annotations on open_image, which would be nice to improve here. Although this can be OK for more trivial snippets like this (opinions differ on what is trivial), it would still be nice to credit add-on authors in these cases. Even features enabled by seemingly trivial code can take a while to ideate, develop, and test, so leaving licensing and CLA requirements aside, I think it's only fair to maintain that continuity of credits going from add-on feature to native feature.

Also I'm not sure if the implementation needs to be as complex as it was here. I wrote this ≈7 years ago, and at that point the OS-specific calls might have been to work around Qt bugs which are no longer present today. In other places in Anki we just delegate picking the correct command to Qt's desktop services API, so I would suggest checking if that is sufficient here as well. If you find that the direct calls are still needed, then it could be advisable not to use subprocess directly as the add-on did, but rather use existing utilities in Anki like call. Similarly, it would probably be good to use the no_bundled_libs context for the QDesktopServices.openUrl call.

[dae]

When I think about experiences with other apps, I think "reveal in Finder/file browser" options seem to be more common than "open in default editor" - as it lets you also do operations on the file, open non-default editors, etc. It's an extra step compared to a separate option to open in the default editor, but it's more flexible if we can't have both.

Please obtain permission before using other people's code in your PRs - when you added your name to the CONTRIBUTORS file, you agreed to do that. The comment you've added gives credit, but now looks like the code has been appropriated, and may make other contributors think they can do the same thing. A better way to approach this would be to ask @glutanimate if he's happy to have that code contributed under the standard BSD-3 license of contributions, and then mention so in your commit message instead of in a code comment.

[user1823]

Thank you all for your inputs. I will be more careful next time. I will try to fix the issues and accommodate the feedback in this PR tomorrow.

[user1823]

This is ready for a second review now.

It's an extra step compared to a separate option to open in the default editor, but it's more flexible if we can't have both.

@dae, does this mean that adding both options would be acceptable?

In other places in Anki, we just delegate picking the correct command to Qt's desktop services API, so I would suggest checking if that is sufficient here as well.

It seems that Qt doesn't provide any option for selecting the desired file in the file manager. So, I have used direct calls. As per your suggestion, I have used call rather than using subprocess directly.

[dae]

Have you copied+pasted this code from somewhere? From a quick read of the Qt docs, it looks like openUrl() should do what we want on all platforms?

[user1823]

For local files, openUrl() opens the file in the default viewer if it can. But, we want it to highlight the file even when there is a viewer available.

[dae]

Sorry, I was under the mistaken impression that it would select the file, not open it directly.

[dae]

Any feature we add should work across all OSes - is there a reason why non-Mac/Windows has been excluded here?

[user1823]

It would definitely be better to make a feature work on all platforms. But, I have no idea how to make it work with all the different file managers in Linux. Even if this doesn't work on Linux, it is still an improvement over what we currently have, especially when you consider that Windows + Mac users comprise most of the Anki's userbase.

[dae]

"We can't do it on Linux easily" is a reasonable answer for now :-) Thanks for clarifying.

[user1823]

It's an extra step compared to a separate option to open in the default editor, but it's more flexible if we can't have both.

@dae, does this mean that adding both options would be acceptable?

Would you accept another PR adding an "Open image" option?

[dae]

Yes, if you feel it's necessary.