Skip to content

✨ Enhances the jukebox CLI with Typer#254

Open
Gudsfile wants to merge 3 commits into
refactor/phase-4-settings-controllerfrom
feature/jukebox-as-typer-cli
Open

✨ Enhances the jukebox CLI with Typer#254
Gudsfile wants to merge 3 commits into
refactor/phase-4-settings-controllerfrom
feature/jukebox-as-typer-cli

Conversation

@Gudsfile
Copy link
Copy Markdown
Owner

@Gudsfile Gudsfile commented May 5, 2026

Part of #234

Summary

Mirrors the jukebox-admin CLI with Typer (introduced in #167).

  • Replace argparse-based CLI with Typer: delete adapters/inbound/config.py and rewrite app.py
  • Move build_settings_service and build_runtime_resolver out of app.py into di_container.py, where all wiring belongs
  • Port all tests from sys.argv patching to typer.testing.CliRunner; add coverage for bad options, unknown subcommands, and CLI-vs-persisted-settings precedence

Test plan

  • uv run pytest all tests pass
  • uv run jukebox --help options render correctly
  • uv run jukebox --version version printed and exits
  • uv run jukebox --sonos-host 1.2.3.4 --sonos-name foo exits with mutual exclusion error
  • uv run jukebox --pause-duration toto exits with not valid type error

@Gudsfile Gudsfile self-assigned this May 5, 2026
@Gudsfile Gudsfile changed the title ✨ Jukebox Typer CLI May 5, 2026
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch from a4b3a46 to ddf61ab Compare May 6, 2026 23:36
@Gudsfile Gudsfile force-pushed the refactor/phase-4-settings-controller branch from a2ba362 to 2667dc6 Compare May 7, 2026 16:22
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch from ddf61ab to 4a9f5be Compare May 7, 2026 16:23
@Gudsfile Gudsfile force-pushed the refactor/phase-4-settings-controller branch from 2667dc6 to d9ef311 Compare May 7, 2026 22:02
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch from 4a9f5be to 936e069 Compare May 7, 2026 22:03
@Gudsfile Gudsfile force-pushed the refactor/phase-4-settings-controller branch from d9ef311 to 2e9f15e Compare May 7, 2026 22:46
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch 2 times, most recently from 9a51055 to 7ab6c4f Compare May 8, 2026 09:23
@Gudsfile Gudsfile force-pushed the refactor/phase-4-settings-controller branch from 2e9f15e to ff6bc8c Compare May 8, 2026 10:12
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch 8 times, most recently from 83b3ef7 to 51587b1 Compare May 9, 2026 17:19
@Gudsfile
Copy link
Copy Markdown
Owner Author

Gudsfile commented May 9, 2026

screen displaying the results of command lines

@Gudsfile Gudsfile changed the title ✨ Jukebox Typer CLI ✨ Enhances the jukebox CLI with Typer May 9, 2026
@Gudsfile Gudsfile force-pushed the refactor/phase-4-settings-controller branch from ff6bc8c to 484d589 Compare May 9, 2026 17:43
@Gudsfile Gudsfile force-pushed the feature/jukebox-as-typer-cli branch from 51587b1 to 76b985b Compare May 9, 2026 17:44
@Gudsfile Gudsfile marked this pull request as ready for review May 9, 2026 17:52
@msgerbush
Copy link
Copy Markdown
Collaborator

This is looking great!!

Comment thread jukebox/app.py
app = typer.Typer(help="Play music on speakers using NFC tags", invoke_without_command=True)


@app.callback()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be a command, rather than a callback? I believe callback is only used when you're expecting subcommands, and the main CLI application is just a wrapper for those commands. https://typer.tiangolo.com/tutorial/commands/callback/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you look at the help text, you'll see it's outputting Usage: jukebox [OPTIONS] COMMAND [ARGS]... even though jukebox has no subcommands

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.

2 participants