Skip to content
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

swaync: Add onChange #6233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LemmusLemmus
Copy link

Calls swanyc-client with the argument reload-css/reload-config when config/CSS is changed.

Description

I have simply added calls to swaync --reload-css and swaync --reload-config for when the config or CSS are changed, so that changes take effect immediately.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

Maintainer CC

@abayomi185

@LemmusLemmus
Copy link
Author

LemmusLemmus commented Dec 25, 2024

Hmm, my implementation might not be the best way to do this. Judging by other usages of onChange in this repo, maybe one should use:

  • lib.mkIf (cfg.package != null)?
  • Maybe check if swaync is enabled to begin with?
  • Maybe one should append || true at the end of the command?

I am not sure what the preferred method is though.

Call swanyc-client with the argument reload-css/reload-config when config/CSS is changed.
@rycee
Copy link
Member

rycee commented Dec 26, 2024

Thanks for the contribution, you could give

diff --git a/modules/services/swaync.nix b/modules/services/swaync.nix
index 5f813af3..8512d794 100644
--- a/modules/services/swaync.nix
+++ b/modules/services/swaync.nix
@@ -98,6 +98,11 @@ in {
         PartOf = [ "graphical-session.target" ];
         After = [ "graphical-session-pre.target" ];
         ConditionEnvironment = "WAYLAND_DISPLAY";
+        X-Restart-Triggers = lib.mkMerge [
+          [ config.xdg.configFile."swaync/config.json".source ]
+          (lib.mkIf (cfg.style != null)
+            [ config.xdg.configFile."swaync/style.css".source ])
+        ];
       };
 
       Service = {
diff --git a/tests/modules/services/swaync/swaync.nix b/tests/modules/services/swaync/swaync.nix
index ceb3bf06..777e0d8c 100644
--- a/tests/modules/services/swaync/swaync.nix
+++ b/tests/modules/services/swaync/swaync.nix
@@ -10,8 +10,11 @@
   };
 
   nmt.script = ''
+    serviceFile=home-files/.config/systemd/user/swaync.service
+    serviceFile=$(normalizeStorePaths $serviceFile)
+
     assertFileContent \
-      home-files/.config/systemd/user/swaync.service \
+      $serviceFile \
       ${
         builtins.toFile "swaync.service" ''
           [Install]
@@ -29,6 +32,7 @@
           Description=Swaync notification daemon
           Documentation=https://github.com/ErikReider/SwayNotificationCenter
           PartOf=graphical-session.target
+          X-Restart-Triggers=/nix/store/00000000000000000000000000000000-config.json
         ''
       }
   '';

a try for a more generic solution.

@Lillecarl
Copy link
Contributor

@LemmusLemmus While your change would work great for people running home-manager through the home-manager CLI that runs activationscripts within your shell. Many people activate through the home-manager nixos module, so relying on systemd as @rycee suggests is the better option, give it a try!

@LemmusLemmus
Copy link
Author

LemmusLemmus commented Dec 30, 2024

Sorry for the delay, I am not dismissing the suggestion :)

I might mention that I am using home-manager as a NixOS module (through flakes).

After removing my changes and copying the changes that rycee suggested, nothing seems to happen when I rebuild with a different style? I am probably doing something wrong though, or could it perhaps be that it restarts swaync but not swaync-client?

I assume that X-Restart-Triggers should restart the service. Would reloading it not be better? Although the swaync service seems to be implemented such that it simply calls --reload-config and --reload-css:

ExecReload=@bindir@/swaync-client --reload-config ; @bindir@/swaync-client --reload-css

This ExecReload part is not present in the service definition in home-manager swaync.nix (adding it did not seem to help). The only other difference is that swaync.nix uses graphical-session-pre.target whereas swaync.service.in uses graphical-session.target, dunno why, not that it matters.


By the way, the home manager manual seems to suggest that it should be enough to run
home-manager --override-input home-manager ~/devel/home-manager to start contributing, however I do not have the command home-manager, and instead I have run nix flake lock --override-input home-manager ~/devel/home-manager, as well as changed the home-manager.url in my flake.nix to point at the home-manager directory. This seems to work, but is this the correct way to do it? (If so, maybe it would be worth mentioning in the manual).

@Lillecarl
Copy link
Contributor

Lillecarl commented Dec 30, 2024

Regarding home-manager overrides to start contributing. There's a thousand ways to do it.

The easiest way when developing a single module might be setting

disabledModules = [ "path/in/hm/to/module.nix" ];

Then copying the entire thing into your own config folder, start hacking away and copy it into HM when it's ready for some kind of review.

I'm LilleCarl at Matrix.org, happy to help anyone who's got the energy to contribute! 😄

EDIT: I can't test your changes as all my only machine is a Chromebook, so I don't run any Wayland apps 😢
In NixOS we have X-Reload-Triggers that can trigger systemd reload commands, we don't have those in home-manager yet. Yes I think reloading is nicer, than restarting when possible. If it works I can't argue with it.

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.

3 participants