open-in-mpv: respect the player's `executable` config

uninsane commented 9 months ago

Description

config.yml allows to specify executable distinct from player name:

players:
  mpv:
    name: mpv
    executable: my-mpv-wrapper
    ...

before, the executable setting would be ignored:

$ open-in-mpv 'mpv:///open?url=https%3A%2F%2Fyoutu.be%2FdQw4w9WgXcQ'
2024/02/15 08:27:41 /usr/bin/mpv https://youtu.be/dQw4w9WgXcQ

after this patch, it's respected:

open-in-mpv 'mpv:///open?url=https%3A%2F%2Fyoutu.be%2FdQw4w9WgXcQ'
2024/02/15 08:27:53 /usr/local/bin/my-mpv-wrapper https://youtu.be/dQw4w9WgXcQ

a user might want executable to be respected for any number of reasons:

they want to use open-with-mpv without including mpv on their path
they're running their browser inside a sandbox that requires them to use a helper program to launch mpv (e.g. flatpak'd web browsers, which have to open apps via the org.freedesktop.Portal API)
they want to stylize the browser's mpv separately from the system's mpv in a way which is impractical to do with flags alone

Type of change

Please delete options which are not relevant.

[x] Bug fix (non-breaking change which fixes an issue)

Checklist

Code quality:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
Tests:
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
Other:
- [ ] I have made corresponding changes to the documentation and/or README.md
- [ ] If new dependencies exist, I have checked that they are really necessary and agreed with the maintainers/community (we want to have as few dependencies as possible)

Baldomo commented 9 months ago

Looks good to me. LGTM

uninsane commented 9 months ago

thank you for tagging a new release with this so quickly!

uninsane commented 9 months ago

oh, the release process doesn't seem to update the version field in manifest.json to match the git tags though. it's still pinned to 2.0.1 (extension/Firefox/manifest.json, extension/Chrome/manifest.json, scripts/Info.plist). not sure it really matters for anything outside of e.g. fielding bug reports though.

Baldomo commented 9 months ago

That's intended. That version number only concerns the extension, which has not been updated in any way.

Baldomo / open-in-mpv