-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
radio-active: add module to create config files #7793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
1df94e4
to
321718f
Compare
f7b2370
to
e9e2ad9
Compare
It seems CI/CD will need |
|
||
type = submodule { | ||
options = { | ||
AppConfig = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest the following structure instead:
- Make
programs.radio-active.settings
match thisAppConfig
option. - Move the
aliases
option to the top.
With the above a valid configuration could be
{
enable = true;
settings.limit = 30;
aliases = {
"Deep House Lounge" = "http://198.15.94.34:8006/stream";
};
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration file could have more than AppConfig
bits defined, configs like;
[AppConfig]
# ... stuff for "AppConfig" settings...
... map to a Python dictionary sorta like configs['AppConfig']['player']
-> "ffplay"
So lifting AppConfig
into the config.programs.radio-active.settings
attribute set could lead to future churn when (if) additional sections are added to the configuration file.
Currently is not an issue but, as I'm currently the fork maintainer submitted to NixOS/nixpkgs
for the program, I also hesitate to reduce future flexibility.
edit for example I may wanna add something like [PlayerConfig]
to enable passing extra CLI args
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, then the settings can include the AppConfig
attribute. Can just move the aliases option.
f3800ca
to
a1112cc
Compare
Configuration entry similar to; ```nix programs.radio-active.enable = true; ``` By default `ffplay` is used for recording/playback, but that can be changed by applying either of the following; ```nix programs.radio-active.settings.AppConfig.player = "vlc"; programs.radio-active.settings.AppConfig.player = "mpv"; ``` All other configuration options documented by; https://github.com/deep5050/radio-active?tab=readme-ov-file#default-configs maybe applied under the `AppConfig` attribute set. Finally, the `aliases` attribute set allows for defining key/value pares that will generate a `~/.radio-active-alias` of bookmarked stations, for example something like; ```nix programs.radio-active.settings.aliases = { "Deep House Lounge" = "http://198.15.94.34:8006/stream"; }; ``` ... will result in; ``` Deep House Lounge==http://198.15.94.34:8006/stream ``` WARN: must be applied after NixOS/nixpkgs#441029 WARN: requires update of `flake.lock` too WARN: Darwin hosts may report issues about `pkgs.vlc` Co-authored-by: Robert Helgesson <[email protected]>
a1112cc
to
e8cfe39
Compare
Hmm; recent force-push, after satisfying linter, seems to show that Darwin don't got
Grep-ing about via |
lib.warn '' | ||
Unknown player defined in `config.programs.radio-active.AppConfig.player` | ||
'' pkgs.radio-active; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warnings should go in the warnings
option, that way they are presented consistently. See, e.g.,
home-manager/modules/programs/bun.nix
Lines 51 to 55 in 173a29f
warnings = lib.optional (cfg.package == null && cfg.enableGitIntegration) '' | |
You have enabled git integration for `bun` but have not set `package`. | |
Git integration will not be configured. | |
''; |
WARN:
flake.lock
tooDescription
Configuration entry similar to;
... will produce default configuration
~/.radio-active-configs.ini
By default
ffplay
is used for recording/playback, but that can be changed by applying either of the following;All other configuration options documented by;
https://github.com/deep5050/radio-active?tab=readme-ov-file#default-configs maybe applied under the
AppConfig
attribute set.Finally, the
aliases
attribute set allows for defining key/value pares that will generate a~/.radio-active-alias
of bookmarked stations, for example something like;... will result in;
Checklist
Change is backwards compatible.
Code formatted with
nix fmt
ornix-shell -p treefmt nixfmt deadnix keep-sorted --run treefmt
.Code tested through
nix run .#tests -- test-all
ornix-shell --pure tests -A run.all
.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
If this PR adds an exciting new feature or contains a breaking change.