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

Fixes for "Generate M3U playlist" #245 #325

Merged
merged 2 commits into from
Jan 5, 2024
Merged

Conversation

xjiro
Copy link
Contributor

@xjiro xjiro commented Mar 3, 2022

Fixes #245

The default playlist location also works for multiple album selections.

Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

Thanks for this. The changes make sense and definitely improve the usefulness of this plugin.

b_filename, b_selected_format = QtWidgets.QFileDialog.getSaveFileName(
None, "Save new playlist",
os.path.join(current_directory, default_filename),
"Playlist (*.m3u8 *.m3u)"
)
if b_filename:
filename = string_(b_filename)
filename = str(b_filename)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if that str conversion is actually needed anymore here. Is it?

plugins/playlist/playlist.py Show resolved Hide resolved
Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

@xjiro Can you please bump the plugin version to 1.2? That's needed before this PR can be

Also about your change replacing string_ with str, I think this str call is not needed at all there. This function was used to convert QByteArray or bytes to a string. But looking at other calls to e.g. QFileDialog.getSaveFileName in the Picard code this conversion no longer is necessary here.

Copy link
Member

@phw phw left a comment

Choose a reason for hiding this comment

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

I rebased this PR, which essentially undid the str conversions (which got removed separately in 239279c )

@phw phw merged commit 72d7753 into metabrainz:2.0 Jan 5, 2024
5 checks passed
@phw phw removed the need more work label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fixes for "Generate M3U playlist"
2 participants