Skip to content
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

Add list-tracks command #46

Merged
merged 3 commits into from Feb 5, 2020
Merged

Add list-tracks command #46

merged 3 commits into from Feb 5, 2020

Conversation

geigerzaehler
Copy link
Owner

Closes #45

Add a list-tracks command that prints all tracks that are part of a collection.

@sindreruud could you have a look at this and check if this would work for you?

Add a list-tracks command that prints all tracks that are part of a
collection.

Closes #45
Copy link
Collaborator

@wisp3rwind wisp3rwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me aside from the inline comments I left. I'm not sure how useful this command will be to solve @sindreruud's usecase, but in any case, I'm fine with adding this command. In particuar I agree that actually writing m3u's is not really in-scope for this plugin (unless it is very easy to hook into importfeeds and let it do all the work).

Comment on lines 48 to 49
beets.config[beets.library.Item._format_config_key].set(
options.format)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should use beets.ui._set_format, see the usage in beets.ui.add_format_option. This should also fix the type errors on Python 2.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my initial idea, too. However, _set_format is an internal method on the CommonOptionsParser object and I’d like to avoid constructing such an object if we don’t need it. I tackled the Python 2.7 error by using decargs.

Comment on lines +94 to +96
description="""
List all tracks that are currently part of an alternative
collection""",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does argparse strip the additional linebreak here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. (I had to insert the linebreak because of flake8)

README.md Outdated
The `--format` option accepts a [beets path format][path-format] string that is
used to format each track.

[path-format]: https://beets.readthedocs.io/en/v1.4.9/reference/pathformat.html
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better change this to the latest readthedocs URL.

https://beets.readthedocs.io/en/latest/reference/pathformat.html

@wisp3rwind
Copy link
Collaborator

Looking good 🎉 I'm just gonna go ahead and merge, this is a reasonable command to have whether or not it ultimately solves @sindreruud's usecase.

@wisp3rwind wisp3rwind merged commit b4a7865 into master Feb 5, 2020
@geigerzaehler geigerzaehler deleted the list-command branch February 6, 2020 10:31
@wisp3rwind
Copy link
Collaborator

btw, the recent travis failures on Python 3.9 were caused by this bug: https://travis-ci.community/t/python-development-versions-no-longer-include-pip-due-to-virtualenv-20-x/7180 which is fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create m3u for newly imported tracks
2 participants