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

plugins: add a plugin to export a playlist to a folder #2307

Merged
merged 8 commits into from Mar 11, 2017

Conversation

ptitjes
Copy link
Collaborator

@ptitjes ptitjes commented Mar 8, 2017

This is a rough cut.
Please review and comment as to make it better.

@ptitjes ptitjes requested review from lazka and declension March 9, 2017 19:12
@declension
Copy link
Member

@ptitjes I'll try to have a look tonight.

(FYI this kind of feature (albeit a less pretty version) can often be achieved by the Custom Commands plugin)

@ptitjes
Copy link
Collaborator Author

ptitjes commented Mar 10, 2017

@declension Thx.

FYI this kind of feature (albeit a less pretty version) can often be achieved by the Custom Commands plugin

I see. I did not this Custom Commands feature. But indeed, I'm glad to be able to select the destination folder and change the pattern at export :)

Copy link
Member

@declension declension left a comment

Choose a reason for hiding this comment

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

A few comments inline but generally LGTM

pattern_label.set_xalign(0.0)
box.pack_start(pattern_label, True, True, 0)

self.pattern_entry = Gtk.Entry(text=pattern)
Copy link
Member

Choose a reason for hiding this comment

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

a qltk.UndoEntry is slightly nicer IMO, or even ValidatingEntry here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at the PatternCompiler/PatternParser/PatternLexer trio, I fail to see where it can fail. They seem to be bullet-proof even with ill-written patterns. What would validate a ValidatingEntry then ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that logic has changed a lot now and is more resilient to user errors (i.e. assumes more things are just text now). Probably needs some rethought, but there are too many use cases in the wild now for significant changes without breaking lots of people's set ups.

Anyway I found you can usually break it with # pretty easily - <#foo>

box.pack_start(destination_label, True, True, 0)

frame = Gtk.Frame()
self.directory_chooser = \
Copy link
Member

Choose a reason for hiding this comment

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

FYI we generally try to avoid backslash continuations (some discussion on alternatives, though the PEP itself was rejected: https://www.python.org/dev/peps/pep-3125/)

self.set_response_sensitive(Gtk.ResponseType.OK, has_directory)

pattern_text = self.pattern_entry.get_text()
has_pattern = pattern_text != ""
Copy link
Member

Choose a reason for hiding this comment

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

FYI it's generally considered more Pythonic to use has_pattern = bool(pattern_text), or in the negative case just not pattern_text (also deals with None, etc)

class Config(object):
_config = PluginConfig(__name__)

DEFAULT_PATTERN = "<artist> - <title>"
Copy link
Member

Choose a reason for hiding this comment

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

I recommend perhaps using a tied tag "<artist~title>" as it works better if is either is missing (e.g. in Classical music 0004. - - Sonata K.380.flac")
...unless those filenames themselves needs parsing by something strict I guess, but this is probably not the default case...

hbox.set_border_width(6)
label = Gtk.Label(label=_("Default filename pattern:"))
hbox.pack_start(label, False, True, 0)
entry = Gtk.Entry()
Copy link
Member

Choose a reason for hiding this comment

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

This one more so even perhaps (custom Entry)

* fix som python code and style patterns
* use UndoEntry for pattern entry
* set a better default filename pattern
@declension declension merged commit 3a43d0a into quodlibet:master Mar 11, 2017
@declension
Copy link
Member

Thanks

@ptitjes
Copy link
Collaborator Author

ptitjes commented Mar 11, 2017

Thank you.

@ptitjes ptitjes deleted the export-to-folder branch March 11, 2017 12:18
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.

None yet

2 participants