i3status-rust: init #548
i3status-rust: init #548
Conversation
|
Would I need to write anything to documentation for this? Or is that automatically taken care of? |
There was a problem hiding this comment.
I had these colors from stylix set in my hm configs. But I figured I should add them to the stylix repo for other users of i3status-rust!
Thanks for sharing!
Would I need to write anything to documentation for this? Or is that automatically taken care of?
Options are automatically injected into the documentation.
From 4fdf452b9688b0f4be1d159d3cbdaf326117ad8d Mon Sep 17 00:00:00 2001 From: Zachary Hanham <[email protected]> Date: Sun, 8 Sep 2024 13:43:16 -0400 Subject: [PATCH] i3status-rust: init --- modules/i3status-rust/hm.nix | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 modules/i3status-rust/hm.nix diff --git a/modules/i3status-rust/hm.nix b/modules/i3status-rust/hm.nix new file mode 100644 index 00000000..39d182f8 --- /dev/null +++ b/modules/i3status-rust/hm.nix @@ -0,0 +1,25 @@ +{ config, lib, ... }: + +let + colors = config.lib.stylix.colors.withHashtag; +in +{ + options.stylix.targets.i3status-rust.enable = config.lib.stylix.mkEnableTarget "i3status-rust" true; + + config = lib.mkIf (config.stylix.enable && config.stylix.targets.i3status-rust.enable) { + programs.i3status-rust.bars.default.settings.theme.overrides = with colors; { + idle_bg = base00; + idle_fg = base05; + info_bg = base09; + info_fg = base05; + good_bg = base01; + good_fg = base05; + warning_bg = base0A; + warning_fg = base05; + critical_bg = base08; + critical_fg = base04; + separator_bg = base00; + separator_fg = base05; + }; + }; +}
I did not test this, but LGTM.
|
@trueNAHO As for testing I did substitute my fork in my flake input and it seemed to work well with no errors. As for the color choices, I'm not positive they're correct, but I tried with a few different backgrounds and they looked good. |
There was a problem hiding this comment.
When using base08-base0F as a background, the corresponding foreground should be base00 instead of base05 to guarantee that the colors contrast. This affects info_fg, warning_fg and critical_fg. You might want to test this after making the change since I haven't tried it myself.
Everything else looks good :)
|
@danth |
|
Line 64 in ef81ad9 |
If upstream supports this, it would be a nice addition. |
|
@pborzenkov That's a good point I didn't consider, especially because you can configure multiple i3status-rust bars to match your (potentially differently themed) bars. It should be easy to do it like that just like how you linked above, so I could amend with that change. |
If you mean home manager's I actually forgot that the |
|
OK I amended with danth's color suggestions as well as just making this attrset exist under I tested again locally and it seems to work well. If you check the first amend, I tried to do it with lib.mkIf but it was giving an infinite recursion error. So in the latest amend I just unconditionally add it to config. Not sure if this is OK but I think it is? Updated my flake input and it worked. An example of usage from my configs (near the bottom): { config, lib, pkgs, ... }:
with lib;
let
cfg = config.i3status-rust;
in
{
options.i3status-rust = {
enable = mkEnableOption "i3status-rust";
};
config = mkIf cfg.enable {
programs.i3status-rust = {
enable = true;
bars.default.blocks = [
{
block = "disk_space";
path = "/";
info_type = "available";
interval = 60;
warning = 20.0;
alert = 10.0;
}
{
block = "memory";
format = " $icon $mem_used_percents ";
interval = 2;
}
{
block = "cpu";
interval = 2;
}
{ block = "sound"; }
{
block = "time";
interval = 60;
format = " $timestamp.datetime(f:'%a %m/%d %I:%M %p') ";
}
];
bars.default.settings.theme.overrides = config.lib.stylix.i3status-rust.bar;
};
};
} |
Actually, unconditionally adding behaviour is a problem: #543. Can you try to resolve this infinite recursion error? Btw, the error could be caused by your setup (not the Stylix patch) accessing something in a problematic way. |
|
So the infinite recursion cause is quite weird and I don't fully understand it. It seems that it's caused by the way that stylix defines and uses I don't really understand why, but inlining I think it has something to do with the fact that I'm defining stuff in To see this yourself, you can try these two files for broken: { config, lib, ... }:
let
colors = config.lib.stylix.colors.withHashtag;
mkEnableTarget = config.lib.stylix.mkEnableTarget;
in
{
options.stylix.targets.i3status-rust.enable = mkEnableTarget "i3status-rust" true;
config = lib.mkIf (config.stylix.enable && config.stylix.targets.i3status-rust.enable) {
# Merge this with your bar's theme's overrides with //config.lib.stylix.i3status-rust.bar
lib.stylix.i3status-rust.bar = with colors; {
idle_bg = base00;
idle_fg = base05;
info_bg = base09;
info_fg = base00;
good_bg = base01;
good_fg = base05;
warning_bg = base0A;
warning_fg = base00;
critical_bg = base08;
critical_fg = base00;
separator_bg = base00;
separator_fg = base05;
};
};
}working: { config, lib, ... }:
let
colors = config.lib.stylix.colors.withHashtag;
mkEnableTarget = let
cfg = config.stylix;
in
humanName:
autoEnable:
lib.mkEnableOption
"theming for ${humanName}"
// {
default = cfg.enable && cfg.autoEnable && autoEnable;
example = !autoEnable;
}
// lib.optionalAttrs autoEnable {
defaultText = lib.literalMD "same as [`stylix.autoEnable`](#stylixautoenable)";
};
in
{
options.stylix.targets.i3status-rust.enable = mkEnableTarget "i3status-rust" true;
config = lib.mkIf (config.stylix.enable && config.stylix.targets.i3status-rust.enable) {
# Merge this with your bar's theme's overrides with //config.lib.stylix.i3status-rust.bar
lib.stylix.i3status-rust.bar = with colors; {
idle_bg = base00;
idle_fg = base05;
info_bg = base09;
info_fg = base00;
good_bg = base01;
good_fg = base05;
warning_bg = base0A;
warning_fg = base00;
critical_bg = base08;
critical_fg = base00;
separator_bg = base00;
separator_fg = base05;
};
};
}So I think a solution might involve a refactor to maybe add { config, lib, mkEnableTarget, ... }:But I'm not really sure, and I'm not sure why the infinite recursion happens in the first place and why inlining fixes it. Any advice appreciated. |
|
Full trace for any curious: |
|
Or even instead of |
|
I am reluctant to modifying According to #18, the i3 module is based on the Sway module, which both successfully use the I assume a minor difference between your implementation and the i3 and Sway modules is causing your infinite recursion error. I suggest starting from one of those modules and modifying small parts until the i3status-rust module is done. At a first glance, your module is not using the Btw, I might consider improving this pattern in the future. |
|
I'll look more into it for sure 👍 |
|
Ok so this new amend does work with no recursion. I think it was because if you try to set a value in If anyone could explain why the "broken" one I listed above infinitely recurses, but this amend doesn't, I'd love to know. |
|
I'm not sure that this new amend satisfies #543 or not though, I believe it does? |
There was a problem hiding this comment.
Ok so this new amend does work with no recursion.
Amazing! Thanks for your hard work.
If anyone could explain why the "broken" one I listed above infinitely recurses, but this amend doesn't, I'd love to know.
Not sure either what exactly caused it.
I'm not sure that this new amend satisfies #543 or not though, I believe it does?
Actually, now I am unsure whether lib.stylix should not unconditionally expose its values since they certainly have no side effects. Sorry for the major inconvenience.
Either way, I don't mind merging 495d49d or 4fc7481. @danth, what do you think?
|
@trueNAHO |
|
I've actually discovered an issue user side with 4fc7481 If you want to add something into the stylix provided attrset, you could try: config.lib.stylix.i3status-rust.bar // { separator = ""; }However this won't work, because it merges the second attrset into the result of the lib.mkIf, which leaves the content untouched. (results in This is not an issue with 495d49d so I think this one should be merged instead. |
There was a problem hiding this comment.
I've actually discovered an issue user side with 4fc7481
If you want to add something into the stylix provided attrset, you could try:
config.lib.stylix.i3status-rust.bar // { separator = ""; }However this won't work, because it merges the second attrset into the result of the lib.mkIf, which leaves the content untouched. (results in
{ _type = "if"; condition = true; content = { ... }; separator = ""; }) So the final result does not have the "separator" attribute in it. (Unless you merge it into "content" specifically, but this is really tedious and not immediately obvious)
Good point. Sorry for not noticing upon reviewing.
This is not an issue with 495d49d so I think this one should be merged instead.
Could you override this PR by force pushing 495d49d?
Done! |
|
Np thanks y'all! |
Reviewed-by: NAHO <[email protected]>
Reviewed-by: NAHO <[email protected]>
Reviewed-by: NAHO <[email protected]>
I had these colors from stylix set in my hm configs. But I figured I should add them to the stylix repo for other users of i3status-rust!
Kept it short and sweet but let me know if anything is needed :)