Add "open image" option to editor

[user1823]

No description provided.

[user1823]

The name of the function "openFolder" is misleading because it can also be used to open files. Do you have any suggestion that is clear yet succint?

Edit:
Does openPath look good?

[dae]

openFolder is existing code that may be used by add-ons, so simplest to just leave the name alone.

The code at present is incorrect, and will print an error message when there's no media URL on the clipboard.

[bpnguyen107]

Hey @user1823, I was able to replicate @dae's error. I believe the suggestions I left you should fix those errors. Your open image feature still doesn't work for me though.

Also, I suggest manually testing all your code changes to Anki using ./run if you aren't already. It allows you to catch these bugs!

[user1823]

Also, I suggest manually testing all your code changes to Anki using ./run if you aren't already. It allows you to catch these bugs!

Sorry. But, that is too much for me because I am not really a developer.

Also, because my previous PR (#3412) also includes url = webview.lastContextMenuRequest().mediaUrl(), I wonder why it is a problem in this PR but not in that PR. Or does the same problem exist in that PR too and it was just left undiscovered?

[bpnguyen107]

I don't know. Did you test your previous PR?

[user1823]

Did you test your previous PR?

Sorry, but I didn't (because I don't know how to).

[user1823]

The error should be fixed now (thanks Ben). I would appreciate if someone could test whether the feature works correctly.

[user1823]

Well, I tested this by creating an add-on that patches the function in Anki's latest beta. The open image function seems to work correctly after these changes. However, nothing happens when I click on the show in folder option. I will try to investigate the issue in the show in folder option tomorrow.

[user1823]

With the latest change, the "show in folder" option also works. However, I am now directly using subprocess rather than using call.

[dae]

I've pushed a fix for macOS. The change you've made to call() is not something I can accept, as passing multiple arguments in a single string is a security issue. I'd also expect the previous usage to work. Does the original call not work for you @user1823 @abdnh?

[user1823]

I'd also expect the previous usage to work. Does the original call not work for you

It didn't work. Probably, there is a trivial error but I couldn't find it.

[user1823]

This works for me. Thanks!

[dae]

Damn, I'd intended to merge this in for beta 2. Oh well.