kde(fix): make wallpaper optional and nondestructive

[rkuklik]

Hello.

I went ahead and fixed #809. While at it, I also made the wallpaper image optional, correcting for some incorrect settings in Look and Feel, which will clash with #717 (but should be more correct).

[Flameopathic]

functionally super nice, way better than my solution.

[Flameopathic]

consider changing the commit message.

The scope is meant to indicate which area of the code was changed. Specifying the type of change, such as feat or fix, is not necessary. Dependency updates should use whichever scope they are related to.

https://stylix.danth.me/commit_convention.html

[rkuklik]

Thank you for the review. I forgot that I shouldn't add the change type. Fortunately, the commits usually get squashed with new message on merge.

[Flameopathic]

Thank you for the review. I forgot that I shouldn't add the change type. Fortunately, the commits usually get squashed with new message on merge.

ah, gotchya, nvm then

[trueNAHO]

IIUC, this PR does the following:

• Reorder entries using the new local mergeWithImage function
• Reuse Id and Name variables to avoid hard-coding them

I am slightly confused how this closes #809 and makes the wallpaper image optional. Maybe I missed something obvious...

[Flameopathic]

I don't wanna take the fun or credit from the author, but I really wanna get this merged so here's how I understand it

I am slightly confused how this closes #809 and makes the wallpaper image optional. Maybe I missed something obvious...

the mergeWithImage function merges two attribute sets, strings, or lists based on config.stylix.image. If the image is null, the second attribute set, string, or list isn't merged at all, and all configuration requiring the image is in these second attribute sets, strings, or lists. that makes the image optional.
#809 doesn't seem to be fixed though.

[rkuklik]

Thank you for the lovely explanation, it is more fun for me that my code is understandable by other people. The issue unfortunatelly isn't fixed (originaly it seemed that it was only caused by plasma-apply-wallpaperimage, which is not the case). It may help people who rely on Plasma look and feel and have their autostart scripts ran in a graphical session.

Apart from that, it makes the wallpaper optional. I don't think I can do anything more now, in the issue we agreed that it would be best to ask KDE devs for help, so that we can stop requiring autostart and only use normal activation. I can ask them when I have more time.

I don't think the original issue is within our power to solve, but it does make the wallpaper optional and plasma-apply-wallpaperimage not a requirement for activation, so I would also like this to be merged.

[trueNAHO]

I am slightly confused how this closes #809 and makes the wallpaper image optional. Maybe I missed something obvious...

the mergeWithImage function merges two attribute sets, strings, or lists based on config.stylix.image. If the image is null, the second attribute set, string, or list isn't merged at all, and all configuration requiring the image is in these second attribute sets, strings, or lists. that makes the image optional.

[...]

Thank you for the lovely explanation, it is more fun for me that my code is understandable by other people.

Thanks for the great explanation. Now everything makes senese.

Somehow I missed the

stylix/modules/kde/hm.nix

Lines 24 to 25 in 28a9d61

	if image == null then
	default

branch everytime I re-read the code...