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

Addresses hook.py ability to handle non-string item properties #2742

Merged
merged 4 commits into from
Nov 22, 2017

Conversation

broddo
Copy link
Contributor

@broddo broddo commented Nov 20, 2017

This is a simple fix for #2740 that catches an AttributeError if the value returned by convert_field() doesn't have a decode() method.

@sampsyo
Copy link
Member

sampsyo commented Nov 20, 2017

Thanks for getting this started! How about something like this to make it clear what types are involved?

if isinstance(converted, bytes):
    return converted.decode(self._coding)
return converted

Would that cover the same set of cases?

@broddo
Copy link
Contributor Author

broddo commented Nov 20, 2017

That certainly looks better than my approach of "break the rules, then ask for forgiveness" approach! I'll run some tests on the various item properties and get back to you.

@broddo
Copy link
Contributor Author

broddo commented Nov 21, 2017

Yeah, that works just fine. A config file using this:

    hooks:
        - event: write
          command: echo "writing track {item.title}, number {item.track} and replay track gain of {item.rg_track_gain}"

results in an output like this:

writing track Moonshine, number 12 and replay track gain of -9.07

So it looks like its handling ints and floats properly now.

@sampsyo
Copy link
Member

sampsyo commented Nov 21, 2017

Perfect! Thank you! Would you mind adding an entry to changelog.rst summarizing the fix?

Also, I know this is really small, but there's some extra whitespace that Travis is complaining about: https://travis-ci.org/beetbox/beets/jobs/305090699#L1099

@sampsyo
Copy link
Member

sampsyo commented Nov 22, 2017

This looks perfect! ✨ Thank you again!

@sampsyo sampsyo merged commit 9ee643f into beetbox:master Nov 22, 2017
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.

2 participants