-
-
Notifications
You must be signed in to change notification settings - Fork 2k
way-displays: support stateful configuration file #7138
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
base: master
Are you sure you want to change the base?
Conversation
77a58f7
to
1d120e7
Compare
e83d47a
to
749da99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Tested the statefulConfiguration use case locally!
} | ||
cfg.settings | ||
]); | ||
home.packages = [ cfg.package ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package being available in the environment is aligned with the stateful workflow but I think it can be useful for the static case as well so I did not make it conditional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could be useful, but see my question about auto-starting the service.
Also, I'm not sure I understand what upside this has compared to just not creating a config file? Wouldn't that have much the same effect? (Except for not starting the service, which I don't understand why you would not want to do.)
749da99
to
ef8b9f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for prompting me to review whether the service could be autostarted. I can...
wayland.windowManager = lib.mapAttrs (name: _: { | ||
systemd.variables = [ "XDG_VTNR" ]; | ||
}) options.wayland.windowManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if we're allowed to do such a thing but I'm willing to be so bold.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is XDG_VTNR ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be found in pam_systemd(8)
. way-displays uses it. The daemon seems to work without it defined, but then the client fails. So for the static configuration workflow it does not seem necessary but for the stateful one it is.
xdg.configFile."way-displays/cfg.yaml" = mkIf (cfg.settings != null) { | ||
source = yaml.generate "way-displays-config.yaml" (mergeSets [ | ||
{ | ||
CALLBACK_CMD = "${pkgs.libnotify}/bin/notify-send \"way-displays \${CALLBACK_LEVEL}\" \"\${CALLBACK_MSG}\""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Losing this sucks. It's like... there should be both a configuration file and a state file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a request for way-displays I guess
ef8b9f1
to
7703bc7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I've tried this out locally now and it seems to work as expected.
Description
This is good for when displays are not considered part of the home configuration and are instead considered ephemeral.
Great for laptops but I use it for my desktop as well.
Checklist
Change is backwards compatible.
Code formatted with
nix fmt
ornix-shell -p treefmt nixfmt-rfc-style keep-sorted --run treefmt
.Code tested through
nix-shell --pure tests -A run.all
or
nix build --reference-lock-file flake.lock ./tests#test-all
using Flakes.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC
@jolars