treewide: deprecate manual targets.${target}.useWallpaper.enable options #2084
Conversation
trueNAHO
added
the
backport: release-25.11
stylix-automation
bot
added
topic: nixos
| { imageScalingMode }: | ||
| { | ||
| programs.regreet.settings.background.fit = lib.mkIf cfg.useWallpaper ( | ||
| programs.regreet.settings.background.fit = |
There was a problem hiding this comment.
Unlike before, this change causes programs.regreet.settings.background.fit to be declared when targets.regreet.image.enable is enabled and targets.regreet.imageScalingMode.enable is disabled. Is this a safe change, assuming upstream should not access the background fitting type when there is no background? Otherwise, this group should be merged with the other { image } group into { image, imageScalingMode }.
There was a problem hiding this comment.
I feel like merging them would make more sense, setting the wallpaper scaling but not the wallpaper image seems silly.
There was a problem hiding this comment.
I feel like merging them would make more sense, setting the wallpaper scaling but not the wallpaper image seems silly.
Yes, but this makes this group artificially more restrictive than necessary. For example, the entire { image, imageScalingMode } group would be disabled when imageScalingMode is disabled, despite image being enabled and desired.
I would say this PR is ready to merge as-is.
trueNAHO
force-pushed
the
treewide-deprecate-manual-targets-target-use-wallpaper-enable-options
branch
from
7e76229 to
47e1eb9
Compare
Deprecate manual targets.${target}.useWallpaper.enable options with
generated targets.${target}.image.enable options, following commit
953c3fb ("stylix/mk-target: generate options for configuring
safeguarded arguments").
trueNAHO
force-pushed
the
treewide-deprecate-manual-targets-target-use-wallpaper-enable-options
branch
from
47e1eb9 to
0a2ffb6
Compare
| { imageScalingMode }: | ||
| { | ||
| programs.regreet.settings.background.fit = lib.mkIf cfg.useWallpaper ( | ||
| programs.regreet.settings.background.fit = |
There was a problem hiding this comment.
I feel like merging them would make more sense, setting the wallpaper scaling but not the wallpaper image seems silly.
trueNAHO
left a comment
trueNAHO
left a comment
There was a problem hiding this comment.
I have not tested this code and merely assume this works as expected.
The following tests successfully build:
-
diff --git a/modules/regreet/testbeds/regreet.nix b/modules/regreet/testbeds/regreet.nix index a4ec92ae..618b32fc 100644 --- a/modules/regreet/testbeds/regreet.nix +++ b/modules/regreet/testbeds/regreet.nix @@ -1 +1,4 @@ -{ programs.regreet.enable = true; } +{ + stylix.targets.regreet.useWallpaper = true; + programs.regreet.enable = true; +}
-
diff --git a/modules/regreet/testbeds/regreet.nix b/modules/regreet/testbeds/regreet.nix index a4ec92ae..388498ee 100644 --- a/modules/regreet/testbeds/regreet.nix +++ b/modules/regreet/testbeds/regreet.nix @@ -1 +1,4 @@ -{ programs.regreet.enable = true; } +{ + stylix.targets.regreet.useWallpaper = false; + programs.regreet.enable = true; +}
| { imageScalingMode }: | ||
| { | ||
| programs.regreet.settings.background.fit = lib.mkIf cfg.useWallpaper ( | ||
| programs.regreet.settings.background.fit = |
There was a problem hiding this comment.
I feel like merging them would make more sense, setting the wallpaper scaling but not the wallpaper image seems silly.
Yes, but this makes this group artificially more restrictive than necessary. For example, the entire { image, imageScalingMode } group would be disabled when imageScalingMode is disabled, despite image being enabled and desired.
I would say this PR is ready to merge as-is.
2 participants
The following remaining
useWallpaperinstances cannot yet be deprecated because their modules have not yet migrated tomkTarget:I have not tested this code and merely assume this works as expected.This has been tested in #2084 (review).