Skip to content

feat(wrapperModules.quickshell): add 'configDir' option#552

Open
jonas-elhs wants to merge 1 commit into
BirdeeHub:mainfrom
jonas-elhs:quickshell/configDir
Open

feat(wrapperModules.quickshell): add 'configDir' option#552
jonas-elhs wants to merge 1 commit into
BirdeeHub:mainfrom
jonas-elhs:quickshell/configDir

Conversation

@jonas-elhs
Copy link
Copy Markdown

Problem:
The current implementation only allows to link single component files next to the shell.qml file. This makes it impossible to use nested quickshell components and import them via import qs.xxx.xxx

First solution:
remove the auto capitalization of the component names to allow for passing a directory of components or a nested path of components. However because all the files were generated into different /nix/store paths as the shell.qml file using root imports was impossible because these files were not in the same directory.

Solution:
Add configDir option alongside configFile, which if set symlinks the whole directory to the quickshell-config path

jonas-elhs added a commit to jonas-elhs/nix-wrapper-modules that referenced this pull request May 23, 2026
@jonas-elhs jonas-elhs force-pushed the quickshell/configDir branch from 7539683 to 1855827 Compare May 23, 2026 14:21
@ormoyo
Copy link
Copy Markdown
Contributor

ormoyo commented May 23, 2026

Hey, thanks for the feedback, I should've thought of that when I made the quickshell module. I'm just kind of a newbie when it comes to quickshell.

Allowing to link an existing quickshell config directory and allowing modules is a nice and easy addition.

To add to this, I think it is also a good idea to add a declarative option to fix this issue. I'm thinking of adding the option components.<name>.module as a way to declaratively set the subdirectories of the component.

Something like:

components.bar = ''
  <some quickshell component>
'';
components.foo = {
  data = ''
    <some quickshell component>
  '';
  module = "time/notifications";
};

I'll open an extra PR for that

@BirdeeHub
Copy link
Copy Markdown
Owner

BirdeeHub commented May 23, 2026

This was definitely quite the oversight on my part to not check how the imports worked properly.

I think I might like the other general approach a bit better still tho? "use the structured options or ignore all those and use the dir" doesn't feel like the best we can do with this. But at the same time, there are also people who have existing quickshell configs so that is also worth thinking about. So it might still be worth giving some way to do this still which is better than overriding --path yourself? Unsure still. Thank you for making us aware of the issue though, it is appreciated.

@jonas-elhs
Copy link
Copy Markdown
Author

The reason I went with this approach of linking the whole directory is that if I have like 50 qml files, I have to manually tell the wrapper where to put each file. Just to rebuild a structure that already exists in the real directory.

@BirdeeHub
Copy link
Copy Markdown
Owner

BirdeeHub commented May 24, 2026

I understand that.

In the meantime you will have to set flags."--path" = lib.mkForce ./yourdir;

Doing this will disable all the other options in the module, as your implementation does.

But using the configDir model, this module is painfully simple.

{ config, lib, wlib, pkgs, ... }: {
  imports = [ wlib.modules.default ];
  package = pkgs.quickshell;
  flags."--path" = ./mypath;
}

It barely requires a module at all at that point.

We may still add this option, but consider doing it like that for the meantime.


As far as this PR implementation goes, rather than making all the logic conditional, you should just be making your configDir option contain by default the generated path, and then treating config.configDir option as the source of truth you pass to --path instead. The linking is not necessary if all you have is the 1 path, it only prevents you from using things like ~/.config/wherever as the path.

In fact, we will likely still do that with the other PR in addition to the changes there, for the reasons we both have stated.

jonas-elhs added a commit to jonas-elhs/nix-wrapper-modules that referenced this pull request May 24, 2026
@jonas-elhs jonas-elhs force-pushed the quickshell/configDir branch from 1855827 to 2011119 Compare May 24, 2026 07:06
@jonas-elhs jonas-elhs force-pushed the quickshell/configDir branch from 2011119 to fb1a312 Compare May 24, 2026 07:08
@jonas-elhs
Copy link
Copy Markdown
Author

Thanks for your suggestion, this makes the implementation a lot simpler

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