-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added optional argument for play-plugin #1532
Conversation
* New config key "play -> optargs" * New Subcommand argument "-o", "--optargs" * Set position using "{}" within "play -> command"-string
Interesting idea! I like this direction. What do you think about this slightly different design: use an ordinary "pass-through arguments" deal. That is, you could use something like:
and the If you want to avoid typing |
Passing through arguments with possible defaults sounds reasonable. I'll take a look at this once the exams are over, but don't expect any masterpieces. As for coding style, please give me some hints if anything doesn't suite. |
Looking good so far! Thanks for following Travis' advice. 😃 |
Finally had some spare time and thought about your idea, @sampsyo . Unfortunately, optparse doesn't support that right away by design (search
My personal favorite (besides my first idea of replacing optparse with |
* New argument `--optargs` reads string from option * If "{}" is present in the given string, the `optargs` from config-file get inserted at that point.
Thanks for investigating! This looks like a good plan to me, especially in the absence of an optional-argument feature. We can reexamine this when we move to Click (#1484). In the mean time, can I ask you the nitpicky favor of changing the name to Other than that, we need to update the documentation and add a changelog entry. Then this will be ready to merge! |
* as discussed in #1532, with args-parameter, and optionally inserted config-key * updated changelog/docs
to make AppVeyor-builds possible.
Except I made the changelog-entry for version 1.3.14, this one's ready to merge. |
Added optional argument for play-plugin
I've merged a simplified version of this feature. Specifically, there's just a command-line flag now (no Thanks again for contributing this. |
By simplyfing the way that the arguments get inserted into/onto the command-string, you actually make my own work useless for myself. I'm using mpv (a fork of mplayer), and this player wants at most one times a "playlist"-argument directly in front of the playlist file. As specifying an optional argument with the simplified version puts it after the command-string, but in front of the playlist, mpv does'nt know what to do with the latter. I would propose the following: Remove the pre-defined string, but leave the ability to position the argument. Thus, the feature would be usable for mpv/mplayer-people, but very much simplified. |
Thanks for the feedback. The above change restores the ability to insert arguments anywhere. By default, they're still tacked onto the end. |
Summary:
Motivation:
I use beets + play-plugin in connection with mplayer. Sometimes I want to listen to musicin the "natural" order, sometimes shuffled. As play doesn't provide such an option, I thought it would be helpful to implement the possibility to add (at request) some additional arguments to the command string.
Even though this could as well have been archived by shuffling the resulting playlist, I think this option gives more flexibility to the user. It can as well be used to route output to different output, etc.
How it works:
In the command-parameter within the config file, add "{}" to the position where you'd like to add the additional parameter (needed like that, as i.e. mplayers last option has to be "-playlist"), and add the config key "optargs: ". When calling
beet play -o
, the command will be called with inserted optional arguments.I'm open for any kind of corrections and suggestions, as well as for a discussion whether or not this really should be merged.