Skip to content

Feat: Smartplaylist: Add dest_regen option for regenerating destination path #5621

Merged
snejus merged 18 commits intobeetbox:masterfrom
pierreay:pr-feat-smartplaylist-path-regen
Mar 14, 2026
Merged

Feat: Smartplaylist: Add dest_regen option for regenerating destination path #5621
snejus merged 18 commits intobeetbox:masterfrom
pierreay:pr-feat-smartplaylist-path-regen

Conversation

@pierreay
Copy link
Contributor

@pierreay pierreay commented Feb 8, 2025

Problem

  1. When items are imported into the library in a don't copy-move mode (-C -M options), they will be registered inside the Beets database using their original paths. However, during subsequent processing (e.g., a convert operation), a path following the Beets path format can be generated.
  2. When generating playlists using smartplaylist plugin, only the path registered inside the Beets database (the original path) can be use inside the output playlist. This block the compatibility with other plugins.

Solution

I added a a new optional configuration option known as dest_regen (as well as its equivalent dest-regen on the CLI) to regenerate items' path in the generated playlist instead of using the ones of the library, just like a convert or a move operation would have done. This operation will happen before the relative_to and prefix options, which makes sense to do so and not in another order, otherwise this new option (dest_regen)) would overwrite the desired behavior of the other mentioned options (relative_to and prefix). It is then helpful to generate playlists compatible with the convert plugin.

To Do

  • Documentation
  • Changelog
  • Tests. (I may write one in the future if this PR is considered)

@snejus snejus requested a review from Copilot March 20, 2025 21:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new configuration option "dest_regen" (with its CLI alias "--dest-regen") to support regenerating the destination path for playlist items, thereby improving compatibility with the convert plugin.

  • Introduces a configuration flag "dest_regen" with a default of False.
  • Adds a corresponding CLI option "--dest-regen".
  • Updates the playlist update logic to optionally use the item's regenerated destination path.
Files not reviewed (2)
  • docs/changelog.rst: Language not supported
  • docs/plugins/smartplaylist.rst: Language not supported
Comments suppressed due to low confidence (1)

beetsplug/smartplaylist.py:267

  • The new dest_regen functionality is not covered by tests; please add tests to ensure that the regenerated destination path is correctly handled under various configurations.
dest_regen = self.config["dest_regen"].get()

@pierreay pierreay force-pushed the pr-feat-smartplaylist-path-regen branch from fd61dc5 to f05ac9e Compare March 22, 2025 09:34
@pierreay
Copy link
Contributor Author

Hello @snejus ! :) Rebasing on master and writing of a test to test the dest_regen feature logic is done!

@pierreay pierreay force-pushed the pr-feat-smartplaylist-path-regen branch from 58eeb07 to d0af4c0 Compare May 14, 2025 06:43
@pierreay
Copy link
Contributor Author

Rebased on master for v2.3.0. :) Anything else that I can do?

@snejus snejus requested a review from a team as a code owner October 14, 2025 22:30
@snejus snejus force-pushed the pr-feat-smartplaylist-path-regen branch from d0af4c0 to d296312 Compare October 16, 2025 07:08
Copy link
Member

@snejus snejus left a comment

Choose a reason for hiding this comment

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

@pierreay I apologise for such a late review again...

I've rebased this branch - let me know if you're able to resolve the linting issues. If not, I can take it from here

@JOJ0 JOJ0 added plugin Pull requests that are plugins related and removed review-needed labels Jan 10, 2026
@pierreay pierreay force-pushed the pr-feat-smartplaylist-path-regen branch from d296312 to c15e91e Compare March 5, 2026 16:16
@pierreay
Copy link
Contributor Author

pierreay commented Mar 5, 2026

Hello @snejus , I rebased the branch on master, and added one commit to fix linting issues found with ruff. However, I was unable to fix the documentation formatting with docstrfmt, as I did not had the same result between my computer and what was returned by poe through GitHub Actions. Moreover, it seems that Actions are stuck for an unknown reason -- I see Expected — Waiting for status to be reported on my page.

@snejus
Copy link
Member

snejus commented Mar 6, 2026

GitHub had an incident with Actions last night, but this should be resolved now. Try

git commit --no-edit --amend --date=now

And force pushing, let's see whether it works now?

@pierreay pierreay force-pushed the pr-feat-smartplaylist-path-regen branch 3 times, most recently from 6030cfd to 5aca83a Compare March 8, 2026 14:23
@codecov
Copy link

codecov bot commented Mar 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.55%. Comparing base (2c6f239) to head (78bc2d9).
⚠️ Report is 19 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5621   +/-   ##
=======================================
  Coverage   69.55%   69.55%           
=======================================
  Files         141      141           
  Lines       18494    18498    +4     
  Branches     3025     3026    +1     
=======================================
+ Hits        12863    12867    +4     
  Misses       4995     4995           
  Partials      636      636           
Files with missing lines Coverage Δ
beetsplug/smartplaylist.py 75.89% <100.00%> (+0.50%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pierreay
Copy link
Contributor Author

pierreay commented Mar 8, 2026

Hi @snejus , this branch has been rebased on master and tests / documentation was reworked to fix all errors and adopt last coding conventions / API change since the last work on it (which was may last year ahah). Everything should be ready to merge IMHO, feel free to raise suggestions if any. Thanks. :)

@snejus
Copy link
Member

snejus commented Mar 10, 2026

@pierreay thanks for this! Now that it's all in place I'll have a look at it, starting with Copilot!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Pierre Ayoub and others added 5 commits March 13, 2026 19:17
Test functions inspired from `test_playlist_update_output_extm3u()` in
`test_smartplaylist.py`.
Test successfully passed using:
`poetry run pytest test/plugins/test_smartplaylist.py`
pierreay and others added 12 commits March 13, 2026 19:17
The aforementioned commits introduced a nmuber of changes since I
implemented this test:
- The syntax `self.assertExists(m3u_filepath)` was an old and now invalid
  way of checking existence of a path using assertion, change to `assert
  m3u_filepath.exists()` which now use string instead of bytes
- Use of `Path()` and strings instead of `path.join` and bytes for
  handling directory path
Styling by Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Rephrasing by Copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@snejus snejus force-pushed the pr-feat-smartplaylist-path-regen branch from be3c98c to 49cb2f5 Compare March 13, 2026 19:17
@snejus snejus enabled auto-merge March 13, 2026 19:18
@snejus
Copy link
Member

snejus commented Mar 13, 2026

Ah, I recently added a new CI check which now failed!

auto-merge was automatically disabled March 14, 2026 08:56

Head branch was pushed to by a user without write access

@pierreay
Copy link
Contributor Author

Sure, the changelog is fixed! Ready to be merged ;)

Copy link
Member

@snejus snejus left a comment

Choose a reason for hiding this comment

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

Thanks!

@snejus snejus merged commit 3bcc539 into beetbox:master Mar 14, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin Pull requests that are plugins related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants