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

System commands update for KDE Plasma 6 #130

Merged
merged 3 commits into from
Mar 13, 2024

Conversation

kosantosbik
Copy link
Contributor

No description provided.

@ManuelSchneid3r
Copy link
Member

If i merge this the commands will not work on Plasma 5 right?

@ManuelSchneid3r
Copy link
Member

Probably checking KDE_SESSION_VERSION=5 or 6 might make sense

@kosantosbik
Copy link
Contributor Author

I still use plasma 5.27.4 on my laptop. The commands do work but after using Shutdown or Reboot, the open applications might not restart automatically when you log back in.

@kosantosbik
Copy link
Contributor Author

Do you still want me to add version check?

@ManuelSchneid3r
Copy link
Member

ManuelSchneid3r commented Mar 13, 2024

Do you still want me to add version check?

well if it is necessary, yes. is there some sort of dbus interface documentation? When did KDE change these commands?

@kosantosbik
Copy link
Contributor Author

kosantosbik commented Mar 13, 2024

I'm not sure where it says in the code/documentation but I made the change based on this comment in KDE Discuss. It confirms the KSMServer will be removed and Shutdown API should be used for both plasma version 5 and version 6. I think it is safe to merge.

@ManuelSchneid3r
Copy link
Member

I cant find any documentation, but read several posts from KDE developers online stating that they changed this interface years ago. I dont plan to maintain backward compatibility for more than a few years (two recent ubuntu lts) so i guess this is fine. ty

@ManuelSchneid3r
Copy link
Member

ManuelSchneid3r commented Mar 13, 2024

however please increment the minor version in the plugins cmake lists. if you are too lazy i can put this in the next version of the plugin.

@kosantosbik
Copy link
Contributor Author

No problem at all. Done.

@ManuelSchneid3r
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants