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

[Do not merge] Also remove embedded art if deleted from the library #19

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

wisp3rwind
Copy link
Collaborator

@wisp3rwind wisp3rwind commented Nov 1, 2018

Split out of #17, this PR is relative to #17. This is not really the solution to the problem (see code comment about performance issues), the PR is mostly to track the issue and conserve the code that was written already.

I don't think that this is the way to go because the check for whether artwork needs to be removed currently requires reading metadata from the file in the alternative collection. This is probably (didn't benchmark anything) no faster than simply re-writing all of the tags blindly. So it might be worth considering to simply to tolerate this edge case not being handled (I do not believe that artwork removal is a very common thing to do). One might add a beet alt update --rewrite flag to precisely rewrite tags (including embedded art) for all items in the alt collection.

Another way to handle this might be to use an additional flexattr to track whether the alternative items have embedded art, e.g. item[self.path_key + '.flags'] = 'has_art,'.

@wisp3rwind wisp3rwind mentioned this pull request Aug 21, 2019
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

1 participant