#872: Tag change hooks #1499
#872: Tag change hooks #1499
Conversation
m-urban
force-pushed
the
tag-change-hook
branch
2 times, most recently
from
89e38b7
to
678816f
Compare
|
FYI, although the tests completed successfully, I just noticed, that AppVeyor throws a ton of warnings for unloaded plugins, unrelated to this PR: https://ci.appveyor.com/project/sampsyo/beets/build/176 |
|
Don't worry about AppVeyor; the tests are broken on Windows (see #670). I set up AppVeyor so we can see how close we're getting to passing the tests there, so it's fine to just ignore it for now unless you're specifically working on Windows-related issues. Thanks for opening the PR! I'll merge this as soon as a get a moment. |
|
Any chance to merge this in? :) |
|
Sorry for being slow on this; I'm finishing up a thesis. Should be done this week, after which my beets bandwidth should increase. I've got a just a couple of small comments, after which we can hit the green button. |
| plugin_albums = plugins.album_for_id(album_id) | ||
| for a in plugin_albums: | ||
| plugins.send('trackinfo_received', info=a) | ||
| candidates.extend(plugin_albums) |
There was a problem hiding this comment.
Since we have a loop in this function anyway, let's make it go over the entire candidates list, and eliminate the event in the special _for_mbid functions. A little less code for a little more clarity.
|
No worries, I am sitting in the same thesis boat right now :) |
|
I would love to see this merged. Can I help in any way? |
|
It looks like the loose ends consist of one little code simplification and a couple of documentation clarity enhancements. If you're interested, you can beat one of us to it by opening a revised PR based on this one that adds a couple more commits! That would be awesome. |
- Integrated support for plugins - Added documentation - Updated changelog
m-urban
force-pushed
the
tag-change-hook
branch
from
9de3bd4
to
1708939
Compare
|
Finally found some spare time to finish up the loose ends of this PR. I'm on it! |
|
Awesome! Thanks, @m-urban! ✨ |
- Fixed wrong event being emitted
…s are only called when searching for explicit IDs
|
Ok, let's give this another shot. I needed to find a new spot to emit the events because the hooks.*_for_id -methods are only called when searching for explicit IDs. Any objections, @sampsyo? |
|
Darn, I forgot about |
|
Ok, forget everything I said today. Quick summary: Concerning and your suggestion was:
The problem is that the *_for_mbid functions are sometimes called directly, in which case the events would not be triggered. |
m-urban
force-pushed
the
tag-change-hook
branch
from
e96e67f
to
6e980a9
Compare
|
You're right, of course—thanks for catching that! Eventually, it wold be nice if everything went through a common interface, but that's clearly not the case now. |
|
All merged up! Thank you for taking care of this. ✨ |
|
👍 |
This PR adds the new hooks
albuminfo_receivedandtrackinfo_receivedfor developers who would like to intercept fetched meta data, before they are applied in tag manipulation operations. See #872