POC lyrics to txt

[holy-elbow]

Description

Very simple POC for writing lyrics to txt files in either a user specified directory or in the album directory. I am not familiar with the beets codebase and couldnt get a pytest working for an actual beets database entry, however a custom pytest I wrote to test with given values works as intended. Heavily plagiarized the Translator class for formatting and what to add. Any help would be appreciated.

(...)

To Do

Add .lrc support. Actually make it work with beets database entries

[github-actions]

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.

[snejus]

Thanks for the PR. I think the feature could be useful, but this implementation currently introduces a couple of crash paths that need to be addressed before it can move forward.

The main blocker is that the new writer runs unconditionally whenever lyrics are updated. The default config sets write_to_file.directory to None, so a normal beet lyrics run now reaches Path(self.directory) and raises a TypeError even when the user has not enabled txt output. This should be gated behind an explicit config value so the existing lyrics workflow remains unchanged by default.

There is also an issue with the album_folder mode. beets stores Item.path as bytes, but Path(item.path) does not accept bytes paths. This should use item.filepath instead.

Also, why are you using LyricsRequestsHandler for writing?

One behavior question: should this only write files when new lyrics are fetched, or should it also be able to export lyrics that are already present in the library? The current early return for existing lyrics means it only works as a fetch-time side effect. That may be fine, but I think it should be intentional and covered by tests.

Could you add focused tests for the disabled-by-default case and the album_folder path handling? Also, CI issues need to be addressed.

[codecov]

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.75%. Comparing base (2efc80b) to head (0b924c8).
⚠️ Report is 66 commits behind head on master.

Files with missing lines	Patch %	Lines
beetsplug/lyrics.py	90.62%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6698      +/-   ##
==========================================
+ Coverage   72.57%   72.75%   +0.17%     
==========================================
  Files         162      162              
  Lines       20810    20841      +31     
  Branches     3292     3297       +5     
==========================================
+ Hits        15103    15162      +59     
+ Misses       4982     4950      -32     
- Partials      725      729       +4     

Files with missing lines	Coverage Δ
beetsplug/lyrics.py	84.64% <90.62%> (+5.46%)	⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[holy-elbow]

Thanks for the PR. I think the feature could be useful, but this implementation currently introduces a couple of crash paths that need to be addressed before it can move forward.

The main blocker is that the new writer runs unconditionally whenever lyrics are updated. The default config sets write_to_file.directory to None, so a normal beet lyrics run now reaches Path(self.directory) and raises a TypeError even when the user has not enabled txt output. This should be gated behind an explicit config value so the existing lyrics workflow remains unchanged by default.

There is also an issue with the album_folder mode. beets stores Item.path as bytes, but Path(item.path) does not accept bytes paths. This should use item.filepath instead.

Also, why are you using LyricsRequestsHandler for writing?

One behavior question: should this only write files when new lyrics are fetched, or should it also be able to export lyrics that are already present in the library? The current early return for existing lyrics means it only works as a fetch-time side effect. That may be fine, but I think it should be intentional and covered by tests.

Could you add focused tests for the disabled-by-default case and the album_folder path handling? Also, CI issues need to be addressed.

Hi, thank you for the quick reply! This is my first time not just contributing to beets, but working on the code base at all. This is a feature I have wanted for some time and decided to finally give it a shot. I skimmed through the existing code and did my best to emulate the formatting, as for calling LyricsRequestHandler, I copied the basic init structure of the Translator class and included it just to be safe as I wasnt fully certain if it was needed or not for what I was trying to accomplish.

I have already been using a hack way to create lyric files from the database by printing them to stdout and piping it into a file. I am sure I can come up with a more elegant solution to work inside of the lyrics plugin, as ideally I want to add the option for people who have pre-existing databases and want to quickly generate the files.

Ive pushed an update to my code (hopefully) fixing the issues. I should have time next week to put a bunch of time into testing and fixing any remaining issues.