beets-play: Add the option to pass the "raw" queried pathes to the player command #1578
Conversation
| if multiple_targets: | ||
| command = command + multiple_targets | ||
| else: | ||
| command.append(target) |
There was a problem hiding this comment.
For future sanity, I think we should just bite the bullet and do the refactoring. That is, there would be only a single way to invoke the function, either with a list-of-strings argument (which can, after all, contain only a single string) or with *args. In either case, we will break backwards-compatibility, but there aren't that many call sites that need fixing. Sound OK?
|
Looks like a great addition! Although I'd like to opt for refactoring over compatibility in this case, if that seems reasonable to you. |
|
Seems like some part of the CI failed because github is having response issues. |
|
I added a commit to refactor However there's an issue in the existing code: |
| @@ -735,7 +735,7 @@ def open_anything(): | |||
| return base_cmd | |||
|
|
|||
|
|
|||
| def interactive_open(target, command=None): | |||
| def interactive_open(targets, command=None): | |||
| """Open the file `target` by `exec`ing a new command. (The new | |||
There was a problem hiding this comment.
The docstring should refer to targets (or perhaps the variable should now be named args or something else generic?).
|
Awesome; this is looking good. Could you please add a description of the option to the docs, at which point we'll merge? And yes, great point about
|
|
I'm quite busy lately, but documentation and and some makeup should be small enough for me to complete them in the coming days. It could take me some time however to push the tests both for this PR and #1578. It works with me though if you're willing to merge this even without the tests. I'll just try to be persistent enough to propose them in a following PR. |
I slightly rewrote the play plugin in order to improve the readability and to introduce the "raw" play config option which makes beet simply pass a list of pathes to the play command rather than a playlist.
interactive_open should now be invoked with at least the list of targets and optionally the command to open the targets with. This allows beets-play to pass multiple file paths directly to the configured command. The changes to the existing invocations are pretty trivial in order to comply to this refactor.
passed_to_command -> open_args
|
Well I just ended up doing it right now. Everything ok on your side? |
beets-play: Add the option to pass the "raw" queried pathes to the player command
|
Awesome! Thanks for taking care of this. I added a changelog entry and merged. And thank you in general for all your contributions lately. I'm adding you as a collaborator in case you see anything you need to maintain directly. Feel free to open PRs still, of course, if you want a code review. |
|
Thank you very much to you! I'll try to keep it up despite the summer break coming to a close. |
I slightly rewrote the play plugin in order to improve the readability and to introduce the "raw" play config option which makes beet simply pass a list of paths to the play command rather than a playlist.
This possibility is useful because vlc doesn't automatically expand (m3u) playlist files added to the (vlc) playlist (other topics about this issue). Which means that if you intend to use
beet playseveral times in a row, either you have to use the--no-playlist-enqueuevlc option which means that playback will be interrupted, either the temporary playlist created by beets will probably never be read and expanded.This PR bypasses this issue by simply giving the option to pass the files directly as arguments to the configured command, which in this use-case, allows uninterrupted vlc playback while giving every track its chance to shine (yay!).
For this to work I also had to slightly modify
beets.util.interactive_open. I'm quite sure I kept it backwards compatible and the only other call, bybeet config -estill works perfecly.