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

Badfiles plugin #1568

Merged
merged 3 commits into from Aug 16, 2015
Merged

Badfiles plugin #1568

merged 3 commits into from Aug 16, 2015

Conversation

sampsyo
Copy link
Member

@sampsyo sampsyo commented Aug 15, 2015

A new plugin by @fxthomas!

Before merging, I'd love to have input from @fxthomas about whether I messed anything up. I also have a short to-do list:

  • No configuration should be required to use the defaults.
  • The documentation page could still use some reorganization to match the rest of the docs.

@fxthomas
Copy link
Collaborator

I checked that it works as expected on my library. Some thoughts and questions :

  • Great for the cleanup! I personally have flake8 set for 90 characters and prefer %-interpolation to format, but having an official coding style is much more important than my own preferences ;)
  • What would you change in the documentation?
  • I submitted a small merge request which among others removes the need for configuration to use the defaults and adds color output to make it more readable. If you have thoughts on that, do not hestiate! (Did I use ui.colorize correctly?)

@fxthomas
Copy link
Collaborator

(See #1570 for the PR)

@fxthomas
Copy link
Collaborator

Also, for some reason beets.dbcore crashes on lib.items(args) when I try a regexp query, e.g. :

beet bad title::'^M.*$'

Any idea on why?

(The one in the docs doesn't actually work either, I was checking it when I found this...)

@fxthomas
Copy link
Collaborator

Never mind, looking at other plugins' source I should have used ui.decargs, now fixed in the PR!

@sampsyo sampsyo merged commit cde509a into master Aug 16, 2015
@sampsyo
Copy link
Member Author

sampsyo commented Aug 16, 2015

Awesome; thanks for the additional changes! And double thanks for acquiescing to the style guide, even if 80 columns seems draconian. 😃 I've added you as a collaborator now, so feel free to continue changing things if you like.

The documentation actually seems like it's in fine shape; I'm not sure what I was talking about before.

The colorization stuff looks perfect; great work figuring that out!

Thanks again for the contribution.

@sampsyo sampsyo deleted the badfiles branch October 19, 2015 18:48
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