bspwm: various improvements (#2095)

* bspwm: various improvements

- fixes shell escaping issues and general style issues
- allow reloading the config on-the-fly by exposing bspwmrc to the user

* bspwm: add configuration test

.github/CODEOWNERS

@@ -293,6 +293,9 @@
 
 /modules/services/unison.nix                          @pacien
 
+/modules/services/window-managers/bspwm               @ncfavier
+/tests/modules/services/window-managers/bspwm         @ncfavier
+
 /modules/services/window-managers/i3-sway/i3.nix      @sumnerevans
 /tests/modules/services/window-managers/i3            @sumnerevans
 

modules/services/window-managers/bspwm/default.nix

@@ -5,70 +5,72 @@ with lib;
 let
 
   cfg = config.xsession.windowManager.bspwm;
-  bspwm = cfg.package;
 
-  camelToSnake = s:
-    builtins.replaceStrings lib.upperChars (map (c: "_${c}") lib.lowerChars) s;
+  camelToSnake =
+    builtins.replaceStrings upperChars (map (c: "_${c}") lowerChars);
 
-  formatConfig = n: v:
+  formatMonitor = monitor: desktops:
+    "bspc monitor ${strings.escapeShellArg monitor} -d ${
+      strings.escapeShellArgs desktops
+    }";
+
+  formatSetting = n: v:
     let
-      formatList = x:
-        if isList x then
-          throw "can not convert 2-dimensional lists to bspwm format"
-        else
-          formatValue x;
-
-      formatValue = v:
-        if isBool v then
-          (if v then "true" else "false")
-        else if isList v then
-          concatMapStringsSep ", " formatList v
+      vStr = if isBool v then
+        boolToString v
+      else if isInt v || isFloat v then
+        toString v
       else if isString v then
-          "${lib.strings.escapeShellArg v}"
+        strings.escapeShellArg v
       else
-          toString v;
-    in "bspc config ${n} ${formatValue v}";
+        throw "unsupported setting type for ${n}";
+    in "bspc config ${strings.escapeShellArg n} ${vStr}";
 
-  formatMonitors = n: v: "bspc monitor ${n} -d ${concatStringsSep " " v}";
-
-  formatRules = target: directiveOptions:
+  formatRule = target: directives:
     let
       formatDirective = n: v:
-        if isBool v then
-          (if v then "${camelToSnake n}=on" else "${camelToSnake n}=off")
-        else if (n == "desktop" || n == "node") then
-          "${camelToSnake n}='${v}'"
+        let
+          vStr = if isBool v then
+            if v then "on" else "off"
+          else if isInt v || isFloat v then
+            toString v
+          else if isString v then
+            v
           else
-          "${camelToSnake n}=${lib.strings.escapeShellArg v}";
+            throw "unsupported rule attribute type for ${n}";
+        in "${camelToSnake n}=${vStr}";
 
-      directives =
-        filterAttrs (n: v: v != null && !(lib.strings.hasPrefix "_" n))
-        directiveOptions;
-      directivesStr = builtins.concatStringsSep " "
-        (mapAttrsToList formatDirective directives);
-    in "bspc rule -a ${target} ${directivesStr}";
+      directivesStr = strings.escapeShellArgs (mapAttrsToList formatDirective
+        (filterAttrs (n: v: v != null) directives));
+    in "bspc rule -a ${strings.escapeShellArg target} ${directivesStr}";
 
-  formatStartupPrograms = map (s: "${s} &");
+  formatStartupProgram = s: "${s} &";
 
 in {
-  options = import ./options.nix {
-    inherit pkgs;
-    inherit lib;
-  };
+  meta.maintainers = [ maintainers.ncfavier ];
+
+  options = import ./options.nix { inherit pkgs lib; };
 
   config = mkIf cfg.enable {
-    home.packages = [ bspwm ];
-    xsession.windowManager.command = let
-      configFile = pkgs.writeShellScript "bspwmrc" (concatStringsSep "\n"
-        ((mapAttrsToList formatMonitors cfg.monitors)
-          ++ (mapAttrsToList formatConfig cfg.settings)
-          ++ (mapAttrsToList formatRules cfg.rules) ++ [''
+    home.packages = [ cfg.package ];
+
+    xdg.configFile."bspwm/bspwmrc".source = pkgs.writeShellScript "bspwmrc" ''
+      ${concatStringsSep "\n" (mapAttrsToList formatMonitor cfg.monitors)}
+
+      ${concatStringsSep "\n" (mapAttrsToList formatSetting cfg.settings)}
+
+      bspc rule -r '*'
+      ${concatStringsSep "\n" (mapAttrsToList formatRule cfg.rules)}
+
       # java gui fixes
       export _JAVA_AWT_WM_NONREPARENTING=1
       bspc rule -a sun-awt-X11-XDialogPeer state=floating
-          ''] ++ [ cfg.extraConfig ]
-          ++ (formatStartupPrograms cfg.startupPrograms)));
-      configCmdOpt = optionalString (cfg.settings != null) "-c ${configFile}";
-    in "${cfg.package}/bin/bspwm ${configCmdOpt}";
+
+      ${cfg.extraConfig}
+      ${concatMapStringsSep "\n" formatStartupProgram cfg.startupPrograms}
+    '';
+
+    xsession.windowManager.command =
+      "${cfg.package}/bin/bspwm -c ${config.xdg.configHome}/bspwm/bspwmrc";
   };
 }

modules/services/window-managers/bspwm/options.nix

@@ -150,16 +150,16 @@ in {
       type = types.package;
       default = pkgs.bspwm;
       defaultText = literalExample "pkgs.bspwm";
-      description = "bspwm package to use.";
+      description = "The bspwm package to use.";
       example = literalExample "pkgs.bspwm-unstable";
     };
 
     settings = mkOption {
       type = with types;
         let primitive = either bool (either int (either float str));
-        in attrsOf (either primitive (listOf primitive));
+        in attrsOf primitive;
       default = { };
-      description = "bspwm configuration";
+      description = "General settings given to <literal>bspc config</literal>.";
       example = {
         "border_width" = 2;
         "split_ratio" = 0.52;
@@ -170,7 +170,8 @@ in {
     extraConfig = mkOption {
       type = types.lines;
       default = "";
-      description = "Additional configuration to add.";
+      description =
+        "Additional shell commands to be run at the end of the config file.";
       example = ''
         bspc subscribe all > ~/bspc-report.log &
       '';
@@ -179,14 +180,16 @@ in {
     monitors = mkOption {
       type = types.attrsOf (types.listOf types.str);
       default = { };
-      description = "bspc monitor configurations";
+      description =
+        "Specifies the names of desktops to create on each monitor.";
       example = { "HDMI-0" = [ "web" "terminal" "III" "IV" ]; };
     };
 
     rules = mkOption {
       type = types.attrsOf rule;
       default = { };
-      description = "bspc rules";
+      description =
+        "Rule configuration. The keys of the attribute set are the targets of the rules.";
       example = literalExample ''
         {
           "Gimp" = {

tests/default.nix

@@ -123,6 +123,7 @@ import nmt {
     ./modules/services/redshift-gammastep
     ./modules/services/sxhkd
     ./modules/services/syncthing
+    ./modules/services/window-managers/bspwm
     ./modules/services/window-managers/i3
     ./modules/services/window-managers/sway
     ./modules/services/wlsunset

tests/modules/services/window-managers/bspwm/bspwmrc

@@ -0,0 +1,18 @@
+bspc monitor 'focused' -d 'desktop 1' 'd'\''esk top'
+
+bspc config 'border_width' 2
+bspc config 'external_rules_command' '/path/to/external rules command'
+bspc config 'gapless_monocle' true
+bspc config 'split_ratio' 0.520000
+
+bspc rule -r '*'
+bspc rule -a '*' 'center=off' 'desktop=d'\''esk top#next' 'split_dir=north' 'sticky=on'
+
+# java gui fixes
+export _JAVA_AWT_WM_NONREPARENTING=1
+bspc rule -a sun-awt-X11-XDialogPeer state=floating
+
+extra config
+
+foo &
+bar || qux &

tests/modules/services/window-managers/bspwm/configuration.nix

@@ -0,0 +1,39 @@
+{ lib, pkgs, ... }:
+
+with lib;
+
+{
+  config = {
+    xsession.windowManager.bspwm = {
+      enable = true;
+      monitors.focused =
+        [ "desktop 1" "d'esk top" ]; # pathological desktop names
+      settings = {
+        border_width = 2;
+        split_ratio = 0.52;
+        gapless_monocle = true;
+        external_rules_command = "/path/to/external rules command";
+      };
+      rules."*" = {
+        sticky = true;
+        center = false;
+        desktop = "d'esk top#next";
+        splitDir = "north";
+        border = null;
+      };
+      extraConfig = ''
+        extra config
+      '';
+      startupPrograms = [ "foo" "bar || qux" ];
+    };
+
+    nmt.script = ''
+      bspwmrc=home-files/.config/bspwm/bspwmrc
+      assertFileExists "$bspwmrc"
+      assertFileIsExecutable "$bspwmrc"
+      assertFileContent "$bspwmrc" ${
+        pkgs.writeShellScript "bspwmrc-expected" (readFile ./bspwmrc)
+      }
+    '';
+  };
+}

tests/modules/services/window-managers/bspwm/default.nix

@@ -0,0 +1 @@
+{ bspwm-configuration = ./configuration.nix; }