fix: handle kglobalaccel D-Bus name owner properly #984
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed the D-Bus GetNameOwner call to store the result and check if kglobalaccel is available Updated warning message to info when kglobalaccel becomes ready This ensures proper initialization of KWin shortcuts only when kglobalaccel service is actually available Log: Fixed kglobalaccel service detection for KWin shortcuts Influence: 1. Test KWin shortcut functionality with and without kglobalaccel service running 2. Verify that shortcuts work correctly when kglobalaccel starts after the window manager 3. Check that no errors occur when kglobalaccel is not available 4. Test D-Bus signal handling for service availability changes fix: 正确处理 kglobalaccel D-Bus 名称所有者 修改 D-Bus GetNameOwner 调用以存储结果并检查 kglobalaccel 是否可用 当 kglobalaccel 准备就绪时将警告消息更新为信息级别 这确保仅在 kglobalaccel 服务实际可用时正确初始化 KWin 快捷键 Log: 修复了 KWin 快捷键的 kglobalaccel 服务检测 PMS: BUG-344071 Influence: 1. 测试在有和没有运行 kglobalaccel 服务时的 KWin 快捷键功能 2. 验证当 kglobalaccel 在窗口管理器之后启动时快捷键是否正常工作 3. 检查当 kglobalaccel 不可用时不会出现错误 4. 测试服务可用性变化的 D-Bus 信号处理
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts kglobalaccel D-Bus name-owner detection and logging to only initialize KWin shortcuts when the kglobalaccel service is actually available and to log readiness at info level instead of warning. Sequence diagram for Manager.init D-Bus kglobalaccel detectionsequenceDiagram
participant Manager
participant ShortcutManager
participant SessionBus
participant DBusDaemon
participant KGlobalAccelService
Manager->>ShortcutManager: AddSystem(wm)
Manager->>ShortcutManager: AddMedia(wm)
Manager->>SessionBus: Call GetNameOwner(org.kde.kglobalaccel)
SessionBus-->>Manager: owner or error
alt kglobalaccel not ready (error from GetNameOwner)
Manager->>Manager: log Warning("kglobalaccel not ready", err)
Manager->>DBusDaemon: NewDBus(SessionBus)
Manager->>DBusDaemon: InitSignalExt(sessionSigLoop, true)
DBusDaemon-->>Manager: ready to emit NameOwnerChanged
DBusDaemon->>Manager: NameOwnerChanged(name, oldOwner, newOwner)
alt name == org.kde.kglobalaccel and newOwner != ""
Manager->>Manager: log Info("kglobalaccel ready")
Manager->>ShortcutManager: AddKWin(wm)
Manager->>DBusDaemon: RemoveAllHandlers()
else other services or kglobalaccel lost
Manager->>Manager: ignore
end
else kglobalaccel already has owner (no error)
Manager->>Manager: owner string is set
Manager->>ShortcutManager: AddKWin(wm) (existing behavior outside diff)
end
Class diagram for Manager and related D-Bus shortcut initializationclassDiagram
class Manager {
- shortcutManager ShortcutManager
- wm WindowManager
- sessionBus SessionBus
- sessionSigLoop SessionSignalLoop
- logger Logger
- owner string
+ init()
}
class ShortcutManager {
+ AddSystem(wm WindowManager)
+ AddMedia(wm WindowManager)
+ AddKWin(wm WindowManager)
}
class SessionBus {
+ BusObject() DBusObject
}
class DBusObject {
+ Call(method string, flags int, name string) DBusCall
}
class DBusCall {
+ Store(owner *string) error
}
class DBusDaemon {
+ InitSignalExt(loop SessionSignalLoop, matchAll bool)
+ ConnectNameOwnerChanged(handler func(name string, oldOwner string, newOwner string))
+ RemoveAllHandlers()
}
class Logger {
+ Warning(args)
+ Info(args)
}
Manager --> ShortcutManager : uses
Manager --> SessionBus : uses
Manager --> DBusDaemon : configures
Manager --> Logger : logs
SessionBus --> DBusObject : creates
DBusObject --> DBusCall : returns
DBusCall --> Manager : sets owner
DBusDaemon --> Manager : NameOwnerChanged callback
Manager --> ShortcutManager : AddKWin on kglobalaccel ready
Logger <.. Manager : Warning/Info level change
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这段代码进行审查：
改进建议： // owner 用于存储 kglobalaccel 服务的所有者信息
var owner string
err := sessionBus.BusObject().Call("org.freedesktop.DBus.GetNameOwner",
0, "org.kde.kglobalaccel").Store(&owner)
if err != nil {
logger.Warning("kglobalaccel not ready:", err)
dbusDaemon := ofdbus.NewDBus(sessionBus)
dbusDaemon.InitSignalExt(m.sessionSigLoop, true)
dbusDaemon.ConnectNameOwnerChanged(func(name string, oldOwner string, newOwner string) {
if name == "org.kde.kglobalaccel" {
if newOwner != "" {
logger.Info("kglobalaccel ready, owner:", newOwner)
m.shortcutManager.AddKWin(m.wm)
dbusDaemon.RemoveAllHandlers()
} else {
logger.Warning("kglobalaccel lost owner")
}
}
})
} else {
// 直接添加，因为服务已经就绪
logger.Info("kglobalaccel ready, owner:", owner)
m.shortcutManager.AddKWin(m.wm)
}主要改进点：
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- You now store the
ownerstring fromGetNameOwnerbut never use it; either remove the variable andStoreargument or add a check that usesownerto decide whether kglobalaccel is available. - Consider aligning the log level for the initial
kglobalaccel not readymessage with the laterkglobalaccel readyinfo log to avoid treating normal startup ordering as a warning.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You now store the `owner` string from `GetNameOwner` but never use it; either remove the variable and `Store` argument or add a check that uses `owner` to decide whether kglobalaccel is available.
- Consider aligning the log level for the initial `kglobalaccel not ready` message with the later `kglobalaccel ready` info log to avoid treating normal startup ordering as a warning.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
robertkill
approved these changes
Dec 23, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, robertkill The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changed the D-Bus GetNameOwner call to store the result and check if kglobalaccel is available
Updated warning message to info when kglobalaccel becomes ready This ensures proper initialization of KWin shortcuts only when kglobalaccel service is actually available
Log: Fixed kglobalaccel service detection for KWin shortcuts
Influence:
fix: 正确处理 kglobalaccel D-Bus 名称所有者
修改 D-Bus GetNameOwner 调用以存储结果并检查 kglobalaccel 是否可用 当 kglobalaccel 准备就绪时将警告消息更新为信息级别
这确保仅在 kglobalaccel 服务实际可用时正确初始化 KWin 快捷键
Log: 修复了 KWin 快捷键的 kglobalaccel 服务检测
PMS: BUG-344071
Influence:
Summary by Sourcery
Improve kglobalaccel service detection to initialize KWin shortcuts only when the D-Bus name owner is actually available and adjust logging accordingly.
Bug Fixes:
Enhancements: