Skip to content

lyrics: add rest_directory configuration option#6745

Open
hakkilab wants to merge 2 commits into
beetbox:masterfrom
hakkilab:rest-directory-config
Open

lyrics: add rest_directory configuration option#6745
hakkilab wants to merge 2 commits into
beetbox:masterfrom
hakkilab:rest-directory-config

Conversation

@hakkilab

Copy link
Copy Markdown

Description

Fixes #2806

  • Adds a rest_directory configuration option to the lyrics plugin that specifies a directory for ReST output, equivalent to the -r, --write-rest command line argument

  • Documentation.

  • Changelog.

  • Tests.

@hakkilab hakkilab requested a review from a team as a code owner June 15, 2026 10:44
@github-actions github-actions Bot added the lyrics lyrics plugin label Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.86%. Comparing base (73605d5) to head (6080faf).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6745      +/-   ##
==========================================
+ Coverage   74.60%   74.86%   +0.26%     
==========================================
  Files         163      163              
  Lines       20830    20831       +1     
  Branches     3283     3283              
==========================================
+ Hits        15541    15596      +55     
+ Misses       4538     4480      -58     
- Partials      751      755       +4     
Files with missing lines Coverage Δ
beetsplug/lyrics.py 89.98% <100.00%> (+10.44%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hakkilab hakkilab force-pushed the rest-directory-config branch from bdd7a46 to 6ece99d Compare June 16, 2026 04:35
@hakkilab hakkilab force-pushed the rest-directory-config branch from 6ece99d to 478ac8c Compare June 16, 2026 05:49
@hakkilab

Copy link
Copy Markdown
Author

Hi @snejus,
This is my first contribution to this project. I fixed issue #2806 by adding a rest_directory config option to the lyrics plugin. I also added tests and documentation updates for the feature. Would appreciate a review when you have time! Let me know what fixes are needed or if there are any steps needed before review.

@snejus snejus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think these tests are slightly too extensive. Your tests do something similar to TestRestFiles except that you override the configuration and use self.run_command to run lyrics command.

pytest.param([]),
],
)
def test_bad_config(self, lyrics_plugin, bad_config):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's no need to test confuse internals :)

Comment on lines +857 to +861
config_file = tmp_path / "config.yaml"
with config_file.open("w") as f:
f.write(f"lyrics:\n rest_directory: {rest_directory}")
self.config.set_file(config_file)
self.config.read(False, False)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should use self.config to adjust configuration.

Comment on lines +869 to +880
cmd = lyrics_plugin.commands()[0]
cmd_args = [] if arg_path is None else ["-r", arg_path]
opts, args = cmd.parser.parse_args(cmd_args)
cmd.func(lib, opts, args)

if output_path is None:
for item in tmp_path.rglob("*"):
assert item.name != "index.rst"
assert item.name != "conf.py"
else:
assert (output_path / "index.rst").exists()
assert (output_path / "conf.py").exists()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And use self.run_command to run commands!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lyrics lyrics plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lyrics: Configuration option for ReST writing

2 participants