kde: enable custom decorations #772

rkuklik · 2025-01-11T18:27:50Z

Hi.

Fixing Plasma activation in #708 worked bit too well. Every time the theme is applied, it resets all custom theming options. I noticed this with decorations, so I added a new option to override org.kde.breeze with custom string.

I also took the liberty of refactoring the module a bit. To improve module maintenance, I mainly reduced the scope of with expressions and improve color handling.

rkuklik · 2025-01-11T18:44:51Z

I am bit unsure why nixfmt fails, perhaps there is a version mismatch (I used one from newest nixos-unstable).

trueNAHO · 2025-01-12T18:38:59Z

I am bit unsure why nixfmt fails, perhaps there is a version mismatch (I used one from newest nixos-unstable).

We are using the new nixfmt-rfc-style formatter, which is not the same as the older nixfmt formatter.

If the behavior of nixfmt-rfc-style recently changed, we might first have to update the nixpkgs input and re-apply nixfmt-rfc-style in a seperate PR. I will look into this once I have more time (probably in two days).

rkuklik · 2025-01-12T18:43:49Z

That is the one I used, nixfmt nixpkgs-unstable-2024-12-04 to be precise. I can reformat the code, but I would say that it is not necessary, if the style updates in the near future.

trueNAHO · 2025-01-12T19:00:00Z

For reference, the following patch should make the CI pass:

diff --git a/modules/kde/hm.nix b/modules/kde/hm.nix
index 92878a3..44f643c 100644
--- a/modules/kde/hm.nix
+++ b/modules/kde/hm.nix
@@ -33,7 +33,9 @@ let
     types
     ;

-  formatValue = value: if isBool value then if value then "true" else "false" else toString value;
+  formatValue =
+    value:
+    if isBool value then if value then "true" else "false" else toString value;

   formatSection =
     path: data:
@@ -45,7 +47,9 @@ let
       directChildren = partitioned.right;
       indirectChildren = partitioned.wrong;
     in
-    optional (directChildren != [ ]) header ++ directChildren ++ flatten indirectChildren;
+    optional (directChildren != [ ]) header
+    ++ directChildren
+    ++ flatten indirectChildren;

   formatLines =
     path: data:
@@ -72,7 +76,9 @@ let
   # PascalCase is the standard naming for color scheme files. Schemes named
   # in kebab-case will load when selected manually, but don't work with a
   # look and feel package.
-  colorschemeSlug = concatStrings (filter isString (builtins.split "[^a-zA-Z]" colors.scheme));
+  colorschemeSlug = concatStrings (
+    filter isString (builtins.split "[^a-zA-Z]" colors.scheme)
+  );

   colorEffect = {
     ColorEffect = 0;
@@ -326,29 +332,31 @@ in
     };
   };

-  config = mkIf (config.stylix.enable && cfg.enable && pkgs.stdenv.hostPlatform.isLinux) {
-    home = {
-      packages = [ themePackage ];
-
-      # This activation entry will run the theme activator when the homeConfiguration is activated
-      activation.stylixLookAndFeel = hm.dag.entryAfter [ "writeBoundary" ] ''
-        ${activator} || verboseEcho \
-          "Stylix KDE theme setting failed. Note that it only works in an already running Plasma session."
-      '';
-    };
-
-    xdg = {
-      systemDirs.config = [ "${configPackage}" ];
-
-      # This desktop entry will run the theme activator when a new Plasma session is started
-      # Note: This doesn't run again if a new homeConfiguration is activated from a running Plasma session
-      configFile."autostart/stylix-activator.desktop".text = ''
-        [Desktop Entry]
-        Type=Application
-        Exec=${activator}
-        Name=Stylix Activator
-        X-KDE-AutostartScript=true
-      '';
-    };
-  };
+  config =
+    mkIf (config.stylix.enable && cfg.enable && pkgs.stdenv.hostPlatform.isLinux)
+      {
+        home = {
+          packages = [ themePackage ];
+
+          # This activation entry will run the theme activator when the homeConfiguration is activated
+          activation.stylixLookAndFeel = hm.dag.entryAfter [ "writeBoundary" ] ''
+            ${activator} || verboseEcho \
+              "Stylix KDE theme setting failed. Note that it only works in an already running Plasma session."
+          '';
+        };
+
+        xdg = {
+          systemDirs.config = [ "${configPackage}" ];
+
+          # This desktop entry will run the theme activator when a new Plasma session is started
+          # Note: This doesn't run again if a new homeConfiguration is activated from a running Plasma session
+          configFile."autostart/stylix-activator.desktop".text = ''
+            [Desktop Entry]
+            Type=Application
+            Exec=${activator}
+            Name=Stylix Activator
+            X-KDE-AutostartScript=true
+          '';
+        };
+      };
 }

I will resolve this issue when I have more time.

rkuklik · 2025-01-12T19:05:55Z

Thank you.

I will resolve this issue when I have more time.

Do you mean the formatting CI, this PR or both? I am unsure whether I should reformat the code, leave it as is for later or help with anything else.

trueNAHO

The problem is that your code is not formated with --width=80:

stylix/flake.nix

Lines 122 to 125 in 2985ee9

    
           nixfmt-rfc-style = { 
        
             enable = true; 
        
             settings.width = 80; 
        
           };

For reference, the current CI command is:

/nix/store/lch913ar0a3w3f6d63crf5vfvpckddk1-nixfmt-unstable-2024-12-04/bin/nixfmt --width=80

If you do not mind the pre-commit hooks (there is currently no developer shell variant without them, although this could be trivially added), feel free to use the developer shells:

stylix/docs/src/development_environment.md

Lines 1 to 14 in 2985ee9

    
           # Development environment 
        
           To enter the developer shell, run: 
        
           ```console 
        
           nix develop 
        
           ``` 
        
           To automatically enter the developer shell upon entering the project directory 
        
           with [`direnv`](https://direnv.net), run: 
        
           ```console 
        
           direnv allow 
        
           ```

I can reformat the code, but I would say that it is not necessary, if the style updates in the near future. [...] I am unsure whether I should reformat the code, leave it as is for later or help with anything else.

Properly format the code, since the CI already uses the latest nixfmt-unstable-2024-12-04.

rkuklik · 2025-01-18T09:42:29Z

Thank you very much for your help and sorry for the trouble.

trueNAHO · 2025-01-18T14:15:57Z

modules/kde/hm.nix

@@ -161,6 +204,7 @@ let
        lookAndFeelMetadata = builtins.toJSON lookAndFeelMetadata;
        lookAndFeelDefaults = formatConfig lookAndFeelDefaults;
      }
+      # bash


Suggested change

# bash

The treesitter grammar (in neovim, for instance) uses this kind of comment to detect which language is in the string, enabling syntax highlighting and other editor integration, same as fenced code blocks in markdown. Should I remove it nonetheless?

The treesitter grammar (in neovim, for instance) uses this kind of comment to detect which language is in the string, enabling syntax highlighting and other editor integration, same as fenced code blocks in markdown. Should I remove it nonetheless?

Yes.

trueNAHO · 2025-01-18T14:20:59Z

modules/kde/hm.nix

-              "KDE theme setting failed. See `${activateDocs}`"
+          activation.stylixLookAndFeel = hm.dag.entryAfter [ "writeBoundary" ] ''
+            ${activator} || verboseEcho \
+              "Stylix KDE theme setting failed. Note that it only works in an already running Plasma session."


This might be more concise:

Suggested change

"Stylix KDE theme setting failed. Note that it only works in an already running Plasma session."

"Stylix KDE theme setting failed. This only works in a running Plasma session."

modules/kde/hm.nix

trueNAHO · 2025-01-18T14:24:28Z

modules/kde/hm.nix

 in
 {
-  options.stylix.targets.kde.enable = config.lib.stylix.mkEnableTarget "KDE" true;
+  options.stylix.targets.kde = {
+    enable = mkEnableTarget "KDE" true;


Suggested change

enable = mkEnableTarget "KDE" true;

enable = mkEnableTarget "KDE" true;

trueNAHO · 2025-01-18T14:33:45Z

modules/kde/hm.nix

+  inherit (config.lib.stylix)
+    colors
+    mkEnableTarget
+    ;
+  inherit (lib)
+    concatStrings
+    concatStringsSep
+    filter
+    flatten
+    getExe
+    isAttrs
+    isBool
+    isString
+    hm
+    last
+    listToAttrs
+    mapAttrsToList
+    mkIf
+    mkOption
+    optional
+    partition
+    range
+    toHexString
+    types
+    ;


Subject: [PATCH 2/5] kde(refactor): remove top-level with and improve imports

Thanks for reducing the scope of the with expressions.

I am aware that Nixpkgs has the convention of brining everything into scope with inhert statements at the top of the file. However, Stylix currently does not really follow this to any extent. It might be better to apply this pattern treewide in a separate PR. @danth, what do you think?

rkuklik added 4 commits January 11, 2025 18:12

kde(style): reformat with nixfmt-rfc-style

9c67936

kde(refactor): remove top-level with and improve imports

91aa455

kde(options): add custom window decorations option

0f9fb38

kde(color): centralize color format

085308f

rkuklik force-pushed the master branch from 18e4b5b to 085308f Compare January 11, 2025 18:32

trueNAHO reviewed Jan 15, 2025

View reviewed changes

kde(style): fix formatting width

1d43263

trueNAHO requested changes Jan 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kde: enable custom decorations #772

kde: enable custom decorations #772

rkuklik commented Jan 11, 2025

rkuklik commented Jan 11, 2025 •

edited

Loading

trueNAHO commented Jan 12, 2025 •

edited

Loading

rkuklik commented Jan 12, 2025

trueNAHO commented Jan 12, 2025

rkuklik commented Jan 12, 2025

trueNAHO left a comment •

edited

Loading

rkuklik commented Jan 18, 2025

trueNAHO Jan 18, 2025

rkuklik Jan 18, 2025

trueNAHO Jan 18, 2025

trueNAHO Jan 18, 2025

trueNAHO Jan 18, 2025

trueNAHO Jan 18, 2025

	# Development environment

	To enter the developer shell, run:

	```console
	nix develop
	```

	To automatically enter the developer shell upon entering the project directory
	with [`direnv`](https://direnv.net), run:

	```console
	direnv allow
	```

	"Stylix KDE theme setting failed. Note that it only works in an already running Plasma session."
	"Stylix KDE theme setting failed. This only works in a running Plasma session."

	enable = mkEnableTarget "KDE" true;
	enable = mkEnableTarget "KDE" true;

kde: enable custom decorations #772

Are you sure you want to change the base?

kde: enable custom decorations #772

Conversation

rkuklik commented Jan 11, 2025

rkuklik commented Jan 11, 2025 • edited Loading

trueNAHO commented Jan 12, 2025 • edited Loading

rkuklik commented Jan 12, 2025

trueNAHO commented Jan 12, 2025

rkuklik commented Jan 12, 2025

trueNAHO left a comment • edited Loading

Choose a reason for hiding this comment

rkuklik commented Jan 18, 2025

trueNAHO Jan 18, 2025

Choose a reason for hiding this comment

rkuklik Jan 18, 2025

Choose a reason for hiding this comment

trueNAHO Jan 18, 2025

Choose a reason for hiding this comment

trueNAHO Jan 18, 2025

Choose a reason for hiding this comment

trueNAHO Jan 18, 2025

Choose a reason for hiding this comment

trueNAHO Jan 18, 2025

Choose a reason for hiding this comment

rkuklik commented Jan 11, 2025 •

edited

Loading

trueNAHO commented Jan 12, 2025 •

edited

Loading

trueNAHO left a comment •

edited

Loading