-
-
Notifications
You must be signed in to change notification settings - Fork 284
palette: add option to allow swapping colorscheme generator logic #1956
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?
palette: add option to allow swapping colorscheme generator logic #1956
Conversation
cf40aa4 to
5c04fd4
Compare
|
I am going to rebase from stable so I can test in my current system. |
5c04fd4 to
bb72bbd
Compare
bb72bbd to
4c3bf20
Compare
|
Basically I copied over the example code using a hardcoded color to my config setup, built my config then in the config generated there is the etc folder where it generates the html with the colorscheme. It changed the colors. |
902f757 to
cc8a714
Compare
trueNAHO
left a comment
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 am looking to experiment with extracting colors and working with them in the oklch color space so I could get more consistent colors and with better contrast.
Just out of curiosity, did you make any progress on this, and how does it compare to Matugen from #892?
The first step in my vision is to allow this project to let me replace the palette generator logic and this PR has the objective to allow me that.
Although you might be the first one interested in out-of-tree palette generator customizations, this sounds like a neat future proof addition without much additional complexity. I would not mind adding this if @0xda157 and @danth also approve the idea.
| readOnly = true; | ||
| internal = true; | ||
| default = (lib.importJSON cfg.generated.json) // { | ||
| default = validateBase16Palette (lib.importJSON cfg.generated.json) // { |
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.
Would this not be better as a check for paletteGeneratorFunction? Otherwise, an assertion would also be fine.
| default = | ||
| { polarity, image }: | ||
| pkgs.runCommand "palette.json" { } '' | ||
| ${lib.getExe cfg.paletteGenerator} "${polarity}" ${lib.escapeShellArg image} "$out" |
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.
This should be properly wrapped as before:
| ${lib.getExe cfg.paletteGenerator} "${polarity}" ${lib.escapeShellArg image} "$out" | |
| ${lib.getExe cfg.paletteGenerator} \ | |
| "${polarity}" \ | |
| ${lib.escapeShellArg image} \ | |
| "$out" |
| missingColors = lib.filter (c: !(palette ? ${c})) requiredColors; | ||
| in | ||
| if missingColors != [ ] then | ||
| throw "Palette generator output is missing required base16 colors: ${lib.concatStringsSep ", " missingColors}" |
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.
Aside from missing outputs, there may also be undesired outputs. Instead of generating a complex diff, it would be better to simply log both with something like:
let
concatStringsSep = builtins.concatStringsSep ", ";
expected = [/* ... */];
in
palette: let
actual = builtins.attrNames palette;
in
lib.throwIf
(actual != expected)
"stylix: palette generator output expected ${
concatStringsSep expected
}, but got: ${
concatStringsSep actual
}"
palette| ''; | ||
| type = lib.types.functionTo lib.types.package; | ||
| default = | ||
| { polarity, image }: |
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.
Consider sorting the arguments:
| { polarity, image }: | |
| { image, polarity }: |
| Example: | ||
| ```nix | ||
| paletteGeneratorFunction = { polarity, image }: | ||
| let | ||
| color = "ffffff"; | ||
| colormap = { | ||
| base00 = color; | ||
| base01 = color; | ||
| base02 = color; | ||
| base03 = color; | ||
| base04 = color; | ||
| base05 = color; | ||
| base06 = color; | ||
| base07 = color; | ||
| base08 = color; | ||
| base09 = color; | ||
| base0A = color; | ||
| base0B = color; | ||
| base0C = color; | ||
| base0D = color; | ||
| base0E = color; | ||
| base0F = color; | ||
| }; | ||
| in | ||
| pkgs.runCommand "palette.json" {color = "ffffff"; } \'\' | ||
| echo \'\$\{builtins.toJSON colormap}\' > $out | ||
| \'\'; |
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.
This should be in example. Ideally, it implements a reasonable alternative real-world example.
| - `polarity`: The polarity setting ("either", "light", or "dark") | ||
| - `image`: Path to the wallpaper image |
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.
Consider sorting the arguments:
| - `polarity`: The polarity setting ("either", "light", or "dark") | |
| - `image`: Path to the wallpaper image | |
| - `image`: Path to the wallpaper image | |
| - `polarity`: The polarity setting ("either", "light", or "dark") |
|
Actually I didn't worked much on this front of work. But the script would basically convert oklch colors to RGB to apply in apps. |
Signed-off-by: lucasew <[email protected]>
cc8a714 to
be66691
Compare
|
I'd like to merge #892 first |

I am looking to experiment with extracting colors and working with them in the oklch color space so I could get more consistent colors and with better contrast. The first step in my vision is to allow this project to let me replace the palette generator logic and this PR has the objective to allow me that.