Skip to content

Conversation

@devmehtaa
Copy link

@devmehtaa devmehtaa commented Oct 27, 2025

Description

Fixes #5786. convert's playlist option uses original extensions

Added a Fallback for Missing Paths:
Instead of directly calling item.path which may sometimes be empty or uninitialized, the code now uses item.path if populated; otherwise, it falls back to item.destination(...). This prevents potential missing-path errors during playlist entry creation by ensuring every item has a valid file path.

Improved Readability and Style Compliance:
The complex ternary expression was reformatted using implicit line continuation inside parentheses, breaking the line across multiple lines. This improves readability and addresses style/linting issues related to line length limits.

Handled Unicode and Encoding:
Playlist entries are checked to ensure proper encoding by decoding bytes to UTF-8 strings as needed. This avoids regressions with non-ASCII filenames ensuring better compatibility.

To Do

  • Documentation. (If you've added a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
  • Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
  • Tests. (Very much encouraged but not strictly required.)

@devmehtaa devmehtaa requested a review from a team as a code owner October 27, 2025 22:55
@github-actions
Copy link

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Add a fallback to item.destination(...) when building playlist entries in case item.path isn’t populated yet to avoid missing‐path errors.
  • Ensure you’re still handling encoding (bytes vs unicode) for playlist lines to prevent regressions with non-ASCII filenames.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Add a fallback to item.destination(...) when building playlist entries in case item.path isn’t populated yet to avoid missing‐path errors.
- Ensure you’re still handling encoding (bytes vs unicode) for playlist lines to prevent regressions with non-ASCII filenames.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@henry-oberholtzer
Copy link
Member

Hi! Thanks for the PR, can you link the issue this addresses or provide more context to the fix?

@devmehtaa devmehtaa closed this Oct 27, 2025
@devmehtaa devmehtaa reopened this Oct 27, 2025
@devmehtaa
Copy link
Author

@henry-oberholtzer I have edited the description comment at the top. Check it out! and let me know if you need any more detail or changes.

@devmehtaa
Copy link
Author

Lint check is not getting successful for no reason? Can you please go through it once please.

@henry-oberholtzer
Copy link
Member

You'll need to run poe format and it should fix the issue - the failure is a little obtuse to read but it will let you know what it fails on.

Also, looks like commit 933132f removed the test you wrote!

@semohr
Copy link
Contributor

semohr commented Oct 28, 2025

Thank you for the PR!

Since this seems to be your first contribution to beets, you might want to have a quick look at our contribution guide and the developer documentation to get familiar with our workflow and coding conventions.

We're thrilled to have you contributing, welcome to the community!

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.

convert's playlist option uses original extensions

3 participants