Let --audio-format take an ordered list of preferred formats - #57
Merged
chrisuthe merged 2 commits intoSep 15, 2026
Merged
Conversation
The option only ever reordered the advertisement, but the docs described it as holding the player to one format. Accept a comma-separated list, offered first in the given order with the rest of the advertisement still behind it, report every missing entry at startup, and describe it as a preference everywhere.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation, validation, ordering behavior, tests, and documentation are consistent with the stated requirements.
Pull request overview
Extends --audio-format from one preferred format to an ordered preference list while preserving fallback formats.
Changes:
- Parses and validates comma-separated format lists.
- Reorders advertised formats and reports all unavailable preferences.
- Updates tests, help text, configuration examples, and documentation.
File summaries
| File | Description |
|---|---|
src/supported_formats.cpp |
Implements list parsing, formatting, and ordering. |
src/supported_formats.h |
Declares and documents the new APIs. |
src/cli.cpp |
Connects list parsing to CLI/config handling. |
src/cli.h |
Stores ordered format preferences. |
src/main.cpp |
Applies preferences and reports missing formats. |
tests/supported_formats_test.cpp |
Tests parsing and advertisement ordering. |
tests/cli_test.cpp |
Tests CLI list handling and validation. |
tests/config_file_test.cpp |
Tests configuration precedence. |
README.md |
Updates user-facing option documentation. |
packaging/sendspin-cli.conf.example |
Updates the packaged configuration example. |
docs/wiki/Configuration.md |
Documents configuration syntax and semantics. |
docs/wiki/Advanced-Usage.md |
Expands usage guidance. |
docs/ROADMAP.md |
Updates the shipped feature description. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…dio-format-take-an-ordered-list-of # Conflicts: # README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--audio-formatonly ever moved one entry to the front of the advertisedsupported_formatslist, but the README and ROADMAP described it as holding the player to that one format. This makes it a list and describes it as what it is: a preference, not a restriction.--audio-format/audio-formattakes one or more comma-separated specs in priority order, e.g.flac:48000:24:2,pcm:48000:24:2. A single spec behaves exactly as before. A repeated flag or key replaces the list, like every other option.--help, the packaged config example, the wiki (Advanced Usage, Configuration), header andmain.cppcomments, and ROADMAP Publish the draft the release job created, not the tag #24 now all describe the option the same way.Tests cover list parsing, rejection cases, merge order with no lost or duplicated entries, and multiple missing entries, plus the flag and config-file paths. Two existing control-socket tests fail under
ctestonly when the build directory's path is long enough to push the absolutized socket path past the 107-bytesun_pathlimit; they pass from a shorter working directory.