Skip to content

feat(wrapperModules.television): init#528

Open
allen-liaoo wants to merge 1 commit into
BirdeeHub:mainfrom
allen-liaoo:main
Open

feat(wrapperModules.television): init#528
allen-liaoo wants to merge 1 commit into
BirdeeHub:mainfrom
allen-liaoo:main

Conversation

@allen-liaoo
Copy link
Copy Markdown

Wrapper module for Television.
Not sure what is the best/recommended way to handle TOML, so I just imitated what other wrapper modules did.

Copy link
Copy Markdown
Owner

@BirdeeHub BirdeeHub left a comment

Choose a reason for hiding this comment

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

Some improvements about the types of paths things can accept and some convenience things, but it looks good!

description = "Television configuration options.";
};
channels = lib.mkOption {
type = types.lazyAttrsOf (types.either types.path tomlFmtType);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
type = types.lazyAttrsOf (types.either types.path tomlFmtType);
type = types.lazyAttrsOf (types.either wlib.types.stringable tomlFmtType);

'';
};
themes = lib.mkOption {
type = types.lazyAttrsOf (types.either types.path tomlFmtType);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
type = types.lazyAttrsOf (types.either types.path tomlFmtType);
type = types.lazyAttrsOf (types.either wlib.types.stringable tomlFmtType);

output = lib.mkOverride 0 config.configDrvOutput;
${if isPathLike v then null else "content"} = builtins.toJSON v;
"builder" =
if isPathLike v then ''cp ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
if isPathLike v then ''cp ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';
if isPathLike v then ''ln -s ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ln -s allows them to use an impure path without needing to wlib.mkOutOfStoreSymlink (and we also adjusted the type to wlib.types.stringable to allow them to pass drvs and just a regular string in addition to a store path)

output = lib.mkOverride 0 config.configDrvOutput;
${if isPathLike v then null else "content"} = builtins.toJSON v;
"builder" =
if isPathLike v then ''cp ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
if isPathLike v then ''cp ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';
if isPathLike v then ''ln-s ${v} "$2"'' else ''${pkgs.remarshal}/bin/json2toml "$1" "$2"'';

"--cable-dir" = lib.mkIf (config.channels != { }) config.channelsDir;
};
constructFiles = {
generatedConfig = {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is the way we have been handling toml yes.

The mapAttrs you did for the other 2 sets is somewhat more complex because you also wanted to accept paths, but also construct files if not a path.

But it looks good I think.

'';
};
};
config = {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
config = {
config = {
passthru.generatedThemesDir = "${config.wrapper.${config.configDrvOutput}}/${baseNameOf config.themesDir}";
passthru.generatedChannelsDir = "${config.wrapper.${config.configDrvOutput}}/${baseNameOf config.channelsDir}";

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Gives an easier way to access the non-placeholder versions of the dirs of these paths.

Of course they could also grab configFiles.<somename>.outPath to get it but like, this is nicer.

typeName = "TOML";
};
isPathLike =
x: builtins.isPath x || (builtins.isString x && lib.hasPrefix "/" x) || lib.isStorePath x;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
x: builtins.isPath x || (builtins.isString x && lib.hasPrefix "/" x) || lib.isStorePath x;
x: builtins.isPath x || (lib.isStringLike x && ! builtins.isString x) || (builtins.isString x && lib.hasPrefix "/" x) || lib.isStorePath x;

Copy link
Copy Markdown
Owner

@BirdeeHub BirdeeHub May 17, 2026

Choose a reason for hiding this comment

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

This honestly may be worth adding as wlib.isPathLike at some point but that would be a separate PR

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