Skip to content

feat(wrapperModules.quickshell): init#544

Merged
BirdeeHub merged 12 commits into
BirdeeHub:mainfrom
ormoyo:quickshell-wrapper-module
May 23, 2026
Merged

feat(wrapperModules.quickshell): init#544
BirdeeHub merged 12 commits into
BirdeeHub:mainfrom
ormoyo:quickshell-wrapper-module

Conversation

@ormoyo
Copy link
Copy Markdown
Contributor

@ormoyo ormoyo commented May 19, 2026

Wrapper module for the quickshell widget toolkit

@ormoyo
Copy link
Copy Markdown
Contributor Author

ormoyo commented May 19, 2026

So, I'm working on a test for it (something like checking logs).

The problem is it gives me the error (specifically in the test):

Detected locale "C" with character encoding "ANSI_X3.4-1968", which is not UTF-8.
Qt depends on a UTF-8 locale, and has switched to "C.UTF-8" instead.
If this causes problems, reconfigure your locale. See the locale(1) manual
for more information.

I tried changing the LANG and LC_ALL environment variables but it only gave me an extra line:

line 2: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8): No such file or directory

@ormoyo
Copy link
Copy Markdown
Contributor Author

ormoyo commented May 19, 2026

After some back and forth, the test is done

@ormoyo ormoyo force-pushed the quickshell-wrapper-module branch from 1d9839d to 041b89f Compare May 19, 2026 09:58
@zenoli
Copy link
Copy Markdown
Contributor

zenoli commented May 19, 2026

@ormoyo In case your interested: I added an easier way to write tests for this lib: https://birdeehub.github.io/nix-wrapper-modules/md/CONTRIBUTING.html#writing-tests-for-wrappers

Feel free to use it :-)

Edit: After having looked at it I'm not sure if your test is suitable as tlib does not expose the env argument of runCommand.

@ormoyo
Copy link
Copy Markdown
Contributor Author

ormoyo commented May 19, 2026

Well, can't I bake the environment variables to the wrapper itself?

@zenoli
Copy link
Copy Markdown
Contributor

zenoli commented May 19, 2026

I have no idea what these variables do and I never used quickshell 🤷
So I cannot tell if the wrapper should ensure that they are set the way they are in your test, or whether this will interfere with other stuff when baked in.

In case it turns out that there are good reasons for injecting them only for the test, I could also extend the test lib to optionally accept an env in form of an attr and pass it down to runCommand to enable the usecase in your test.

@ormoyo ormoyo force-pushed the quickshell-wrapper-module branch 2 times, most recently from e20614e to c7d5aa4 Compare May 19, 2026 16:48
@ormoyo
Copy link
Copy Markdown
Contributor Author

ormoyo commented May 19, 2026

So, the environment variables were just related to the system locale (like en_US.UTF-8) cause QT, the framework of quickshell, needs those for some reason.

I just needed to set env.<VAR> in the wrapper, nothing complicated. These wrappers already allow to modify the environment variables as akin to pkgs.runCommand env arg. So you don't really need to add an env functionality for the test function @zenoli.

@zenoli
Copy link
Copy Markdown
Contributor

zenoli commented May 19, 2026

Great to see that you made it work!

@BirdeeHub
Copy link
Copy Markdown
Owner

Cool stuff! Will check this out soon!

Comment thread wrapperModules/q/quickshell/module.nix Outdated
wlib.types.file (
{ name, ... }:
{
path = mkOptionDefault config.constructFiles."${name}Component".path;
Copy link
Copy Markdown
Owner

@BirdeeHub BirdeeHub May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this path value isn't actually doing anything.

Can we copy the television module, except wlib.types.linkable and wlib.types.lines instead of tomlFmtType?

https://github.com/BirdeeHub/nix-wrapper-modules/blob/main/wrapperModules/t/television/module.nix

wlib.types.linkable was JUST added you will need to rebase (I generalized something used in that module for us to use here)

But that way, if the value is not absolute-path-like, it will merge like lib.types.lines, but if it is an absolute-path-like value, it will merge like a path

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, things change here so fast... it makes me almost shed a tear 🥹

This type seems just like the right tool for the job.

Wouldn't that type be also useful for configFile where the user would most likely want to touch the configFile? (like this and my fish wrapper)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this seems like a better wlib.types.file type

Copy link
Copy Markdown
Owner

@BirdeeHub BirdeeHub May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably is in most cases where you have an attrset of them tbh...

The thing wlib.types.file offers over lib.types.either wlib.types.linkable lib.types.lines is, predictability in that you don't always have to check if the thing has no \n and starts with a / when consuming it.

Honestly though, I mostly have wlib.types.file because lassulus wrappers did... In cases with just 1 file it works fine to just point path to the generated path so people can override it if they wish, but it is very clumsy to use when you have an attrset of them, especially when you need to either generate a file or link it, and they all need to end up in 1 store dir.

Whereas if you have paths you can provide to the program individually, the file type is nice, because you can map the content to a generated file, set path to that, and then pass the program the path values directly, so that those path values are the authoritative source and the user can set them as they wish

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will review this sometime this evening probably, haven't looked at your updates yet.

@ormoyo ormoyo force-pushed the quickshell-wrapper-module branch from b5189d8 to bc24d14 Compare May 20, 2026 13:33
@ormoyo
Copy link
Copy Markdown
Contributor Author

ormoyo commented May 20, 2026

Weird, the failed check says in the logs that xplr has failed. Not quickshell :/

@BirdeeHub
Copy link
Copy Markdown
Owner

BirdeeHub commented May 20, 2026

Weird, the failed check says in the logs that xplr has failed. Not quickshell :/

occasionally github likes to just close pipes for no reason... It is probably the grep -q, the -q will, very occasionally, make the pipe close early and then its like "yo your pipe failed" even though we only cared about the exit code anyway

The problem is without the -q the test output is 20 million lines of just spam so...

Ill re-run it. On any normal desktop computer, you don't hit this case, they just have process swapping turned up to the max on tiny containers, so occasionally it fails on github despite working locally. Re-running it solves it, almost always just once.

We have a test lib now. We can maybe see about what shell opts to use for people using the test lib. the xplr module tests haven't been converted to use the new test lib yet either though (and it isn't always the xplr module, thats why the mpv tests DONT have -q and print binary content into the test output...)

@BirdeeHub
Copy link
Copy Markdown
Owner

BirdeeHub commented May 20, 2026

diff --git a/wrapperModules/q/quickshell/check.nix b/wrapperModules/q/quickshell/check.nix
index 18dabbdf..db07bb3b 100644
--- a/wrapperModules/q/quickshell/check.nix
+++ b/wrapperModules/q/quickshell/check.nix
@@ -79,7 +79,7 @@ test { wrapper = "quickshell"; } {
       "wrapper should load shell.qml and components" =
         let
           wrapper = baseWrapper.wrap {
-            configFile.content = shellContent;
+            configFile = shellContent;
             components.bar = barContent;
           };
         in
@@ -92,7 +92,7 @@ test { wrapper = "quickshell"; } {
       "wrapper should load external files" =
         let
           wrapper = baseWrapper.wrap {
-            configFile.path = pkgs.writeText "quickshell-test-shell.qml" shellContent;
+            configFile = pkgs.writeText "quickshell-test-shell.qml" shellContent;
             components.bar = pkgs.writeText "quickshell-test-bar.qml" barContent;
           };
         in
diff --git a/wrapperModules/q/quickshell/module.nix b/wrapperModules/q/quickshell/module.nix
index 815a85a3..50aac491 100644
--- a/wrapperModules/q/quickshell/module.nix
+++ b/wrapperModules/q/quickshell/module.nix
@@ -11,31 +11,38 @@ let
     mkDefault
     mkIf
     mkOption
-    mkOptionDefault
     types
     ;
 
+  makeForce = lib.mkOverride 0;
   isLinkable = wlib.types.linkable.check;
 in
 {
   imports = [ wlib.modules.default ];
   options = {
     configFile = mkOption {
-      type = wlib.types.file {
-        path = mkOptionDefault config.constructFiles.generatedConfig.path;
-      };
-      default = { };
+      type = types.either wlib.types.linkable types.lines;
+      default = "";
     };
     components = mkOption {
       type = types.attrsOf (types.either wlib.types.linkable types.lines);
       default = { };
     };
+    generated.output = lib.mkOption {
+      type = lib.types.str;
+      default = config.outputName;
+    };
+    generated.placeholder = lib.mkOption {
+      type = lib.types.str;
+      readOnly = true;
+      default = "${placeholder config.generated.output}/${config.binName}-config";
+    };
   };
 
   config.package = mkDefault pkgs.quickshell;
-  config.flags = {
-    "--path" = "${builtins.placeholder config.outputName}/${config.binName}-config";
-  };
+  config.flags."--path" = config.generated.placeholder;
+
+  config.passthru.generatedConfig = "${config.wrapper.${config.generated.output}}/${config.binName}-config";
 
   config.constructFiles =
     mapAttrs' (
@@ -51,14 +58,17 @@ in
         value = {
           content = mkIf (!linkable) val;
           builder = mkIf linkable ''ln -s ${val} "$2"'';
-          relPath = "${config.binName}-config/${capitalizedName}.qml";
+          output = makeForce config.generated.output;
+          relPath = makeForce "${config.binName}-config/${capitalizedName}.qml";
         };
       }
     ) config.components
     // {
       generatedConfig = {
-        content = config.configFile.content;
-        relPath = "${config.binName}-config/shell.qml";
+        content = mkIf (!isLinkable config.configFile) config.configFile;
+        builder = mkIf (isLinkable config.configFile) ''ln -s ${config.configFile} "$2"'';
+        output = makeForce config.generated.output;
+        relPath = makeForce "${config.binName}-config/shell.qml";
       };
     };

^ Can we make configFile a linkable or lines type too, and then finish copying the television module? Doing it this way makes sure it is all properly grouped in the directory, and also we give a passthru value for people to grab the dir from outside of the module easily.

@BirdeeHub
Copy link
Copy Markdown
Owner

BirdeeHub commented May 20, 2026

Also, not covered in the diff above, all the options need descriptions, and I am not sure why the CI did not catch that. It definitely at one point worked.

@BirdeeHub
Copy link
Copy Markdown
Owner

Awesome thanks :) This looks good now. Sorry I meant to review this yesterday but I got busy unexpectedly.

@BirdeeHub BirdeeHub merged commit 8c90cbe into BirdeeHub:main May 23, 2026
2 checks passed
@jonas-elhs
Copy link
Copy Markdown
Contributor

jonas-elhs commented May 23, 2026

Does this wrapper module support quickshell components nested in subfolders and imported via import qs.modules.xxx? Or do they have to be in the root directory aside the shell.qml?

In my own shell I have them sorted and not all in one place

Edit: I opened #552 to support this

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.

4 participants