Skip to content

Have convert use the new format when deciding path #6105

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Have convert use the new format when deciding path #6105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions beetsplug/convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ def convert_item(
item, original, converted = None, None, None
while True:
item = yield (item, original, converted)
item.format = fmt.upper()
Copy link
Member

@henry-oberholtzer henry-oberholtzer Oct 18, 2025

Choose a reason for hiding this comment

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

I think there's a few side-effects of making this change here, partially as the test failures indicate. Most the tests don't really operate with the item's file type in the path, so they shouldn't fail if a change it made to the way file path behavior is handled.

At line 376, 380, 410, there are calls to should_transcode(item, fmt) -- by overriding the item's format type, I believe it'll now skip conversion as it thinks the incoming file is already the target type.

Adding a line below 359 to restore the item's format to it's original might help this keep the path check logic OK. However, keep_new at 374 will put the original file at the dest path instead, so that's an edge case we'll want to be wary of making this change.

Copy link
Author

@j9ac9k j9ac9k Oct 18, 2025

Choose a reason for hiding this comment

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

Would it be better perhaps to add a method to the Item class to more thoroughly handle cases where formats are changed?

Copy link
Member

@henry-oberholtzer henry-oberholtzer Oct 19, 2025

Choose a reason for hiding this comment

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

Item class is a bit too close to the core code I think for this plugin to have enough weight to get a new method. It's also a representative of the database structure primarily.

Intercepting the path_formats is probably the best option, and replacing the string that represents the file format and replacing it with the format for the target path destination. Probably good to have it be covered by a config option as well, so as to not unexpectedly change behavior.

dest = item.destination(basedir=dest_dir, path_formats=path_formats)

# Ensure that desired item is readable before processing it. Needed
Expand Down
2 changes: 2 additions & 0 deletions docs/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Bug fixes:

- |BeetsPlugin|: load the last plugin class defined in the plugin namespace.
:bug:`6093`
- Fixed convert plugin not taking into account the new format when determining
the target path. :bug:`1360`

For packagers:

Expand Down
Loading