Skip to content
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

Fix plasma activation #708

Merged
merged 5 commits into from
Jan 3, 2025
Merged

Conversation

lordkekz
Copy link
Contributor

Fixes: #422, #350 and #340
Based on: #547

KDE theming uses CLI tools which require D-Bus and Plasma session. Instead of breaking home-manager activation, this PR creates a KDE "AutostartScript" that applies the Global Theme during the startup of Plasma.

It's basically a desktop entry in the autostart directory which has a special tag X-KDE-AutostartScript that tells Plasma to start it early. The X-KDE-AutostartScript is probably not required, since I noticed that plasma-manager is doing something similar without it.

This PR has the same scope as #547 but is better because the AutostartScript will run no matter how home-manager is installed - be it as a NixOS module or standalone or some custom solution. The approach of #547 is flawed in that regard because the systemd user unit needs to depend on the plasma session, but that might be started by many different means.

I have tested it on NixOS 24.11 beta (with Plasma 6.2.3).

Note: The AutostartScript will not be run on every activation, even if it is changed. This means that if you run home-manager switch when the Plasma session is already running, any changes to the Global Theme won't be automatically applied. You can run the script manually from system settings to apply it.
If you want, I can also make the script run on activation (but tolerate failure to avoid breaking activation outside of a Plasma session).

@trueNAHO
Copy link
Collaborator

Great job! Let's hear @rkuklik's opinion.

modules/kde/hm.nix Outdated Show resolved Hide resolved
@rkuklik
Copy link
Contributor

rkuklik commented Jan 2, 2025

Hi, sorry for the delay (on both this and my PR).

I like it, a lot. For the past couple of months I have been using my own hand rolled solution, and switching to this worked like a charm. Apart from my comment and maybe fixing the lint warnings and failing test (I have no idea why it complains), I thing it is ready.

@trueNAHO
Copy link
Collaborator

trueNAHO commented Jan 2, 2025

fixing the lint warnings

For reference, the lint warnings are visible in https://github.com/danth/stylix/pull/708/files.

failing test (I have no idea why it complains)

This should already be resolved by commit a4ed416.

modules/kde/hm.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@rkuklik rkuklik left a comment

Choose a reason for hiding this comment

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

God I have no idea how this review system works. Any idea why GH is still complaining that a review is required?

Edit: now I see that somebody with write access has to approve that (silly me)

@rkuklik
Copy link
Contributor

rkuklik commented Jan 3, 2025

Hey, @trueNAHO, can we merge this?

@trueNAHO trueNAHO enabled auto-merge (squash) January 3, 2025 19:48
Copy link
Collaborator

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

Thanks for resolving these long-lasting issues.

@trueNAHO trueNAHO merged commit e0a41d3 into danth:master Jan 3, 2025
34 of 36 checks passed
pinarruiz pushed a commit to pinarruiz/stylix that referenced this pull request Jan 4, 2025
…anth#708)

Replace the systemd unit with a more robust AutostartScript for theme
application during Plasma startup, ensuring D-Bus and session
availability without breaking Home Manager activation.

Closes: danth#340
Closes: danth#350
Closes: danth#422
Link: danth#708

Co-authored-by: rkuklik <[email protected]>
Tested-by: lordkekz <[email protected]>
Tested-by: rkuklik <[email protected]>
Reviewed-by: NAHO <[email protected]>
@stylix-automation
Copy link

Successfully created backport PR for release-24.11:

stylix-automation bot pushed a commit that referenced this pull request Jan 4, 2025
…708)

Replace the systemd unit with a more robust AutostartScript for theme
application during Plasma startup, ensuring D-Bus and session
availability without breaking Home Manager activation.

Closes: #340
Closes: #350
Closes: #422
Link: #708

Co-authored-by: rkuklik <[email protected]>
Tested-by: lordkekz <[email protected]>
Tested-by: rkuklik <[email protected]>
Reviewed-by: NAHO <[email protected]>
(cherry picked from commit e0a41d3)
trueNAHO pushed a commit that referenced this pull request Jan 4, 2025
…708)

Replace the systemd unit with a more robust AutostartScript for theme
application during Plasma startup, ensuring D-Bus and session
availability without breaking Home Manager activation.

Closes: #340
Closes: #350
Closes: #422
Link: #708

Co-authored-by: rkuklik <[email protected]>
Tested-by: lordkekz <[email protected]>
Tested-by: rkuklik <[email protected]>
Reviewed-by: NAHO <[email protected]>
(cherry picked from commit e0a41d3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kde: home-manager failing to set Plasma wallpaper, not connected to D-Bus server
3 participants