feat(gesture): add KWin interaction logging support#1096
Open
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
feat(gesture): add KWin interaction logging support#1096Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 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 |
Reviewer's GuideAdds KWin-specific gesture interaction logging with shared helpers, including KWin and application package/version context, and updates existing display/input event IDs accordingly. Sequence diagram for KWin multitask view gesture loggingsequenceDiagram
actor User
participant Manager
participant WindowManager
participant GestureEventLogHelpers
participant EventLogPackage
participant DpkgQuery
User->>Manager: doHandle4Or5FingersSwipeUp
Manager->>WindowManager: GetMultiTaskingStatus(display 0)
WindowManager-->>Manager: isShowMultiTask false, isShowDesktop false
Manager->>Manager: toggleShowMultiTasking
Manager-->>User: multitask view shown
Manager->>GestureEventLogHelpers: logMultiTaskViewEvent(eventLaunchTypeTouchPad)
GestureEventLogHelpers->>DpkgQuery: packageVersion(kwin-x11)
DpkgQuery-->>GestureEventLogHelpers: kwinVersion
GestureEventLogHelpers->>EventLogPackage: WriteEventLog(EventLogData Tid=eventTidKWinMultiTaskView)
EventLogPackage-->>GestureEventLogHelpers: true
User->>Manager: handleTouchEdgeMoveStopLeave
Manager->>Manager: showMultiTaskingWithLaunchType(eventLaunchTypeTouchScreen)
Manager->>WindowManager: GetMultiTaskingStatus(display 0)
WindowManager-->>Manager: isShowMultiTask false
Manager->>Manager: toggleShowMultiTasking
Manager->>GestureEventLogHelpers: logMultiTaskViewEvent(eventLaunchTypeTouchScreen)
Sequence diagram for KWin split screen gesture loggingsequenceDiagram
actor User
participant Manager
participant WindowManager
participant GestureEventLogHelpers
participant X11Client
participant EventLogPackage
participant FileSystem
participant DpkgQuery
User->>Manager: doTileActiveWindowLeft
Manager->>WindowManager: TileActiveWindow(display 0, wmTileDirectionLeft)
WindowManager-->>Manager: success
Manager->>GestureEventLogHelpers: logSplitScreenEvent
GestureEventLogHelpers->>X11Client: GetActiveWindow
X11Client-->>GestureEventLogHelpers: win
GestureEventLogHelpers->>X11Client: GetWMClass(win)
X11Client-->>GestureEventLogHelpers: WMClass
GestureEventLogHelpers->>FileSystem: ReadFile(desktopPath)
FileSystem-->>GestureEventLogHelpers: desktopEntry
GestureEventLogHelpers->>DpkgQuery: QueryVersion(packageName)
DpkgQuery-->>GestureEventLogHelpers: appVersion
GestureEventLogHelpers->>DpkgQuery: packageVersion(kwin-x11)
DpkgQuery-->>GestureEventLogHelpers: kwinVersion
GestureEventLogHelpers->>EventLogPackage: WriteEventLog(EventLogData Tid=eventTidKWinSplitScreen)
EventLogPackage-->>GestureEventLogHelpers: true
Class diagram for updated gesture manager and KWin event logging helpersclassDiagram
class Manager {
+doHandle4Or5FingersSwipeUp() error
+doTileActiveWindowLeft() error
+doTileActiveWindowRight() error
+showMultiTaskingWithLaunchType(launchType string) error
+doShowMultiTasking() error
}
class GestureEventLogHelpers {
<<utility>>
+logMultiTaskViewEvent(launchType string)
+logSplitScreenEvent()
+getKWinVersion() string
+getActiveWindowAppInfo() (string, string)
+getWindowAppNameAndVersion(win Window) (string, string)
+getDesktopAppInfo(id string) (string, string)
+readDesktopAppInfo(desktopPath string) (string, string)
+getProcessAppInfo(pid uint32) (string, string)
+packageNameByFile(path string) string
+packageVersion(packageName string) string
-kwinVersion string
-kwinVersionOnce Once
}
class EventLogData {
+Tid int64
+Target string
+Message map~string string~
}
class EventLogPackage {
+WriteEventLog(data *EventLogData) bool
}
class X11Client {
+GetActiveWindow() Window
+GetWMPid(win Window) uint32
+GetWMClass(win Window) WMClass
+GetWMName(win Window) string
}
class FileSystem {
+ReadFile(path string) []byte
+Getenv(key string) string
}
class DpkgQuery {
+QueryOwner(path string) string
+QueryVersion(packageName string) string
}
Manager ..> GestureEventLogHelpers : uses
GestureEventLogHelpers ..> EventLogPackage : writes events
GestureEventLogHelpers ..> X11Client : queries active window
GestureEventLogHelpers ..> FileSystem : reads desktop files
GestureEventLogHelpers ..> DpkgQuery : resolves package info
EventLogPackage ..> EventLogData : writes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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:
- The
packageVersionandpackageNameByFilehelpers invokedpkg-querysynchronously on each call, which may be expensive along the gesture path; consider caching per-package results (similar tokwinVersion) or performing these lookups lazily/off the critical path to avoid blocking UI interactions. logSplitScreenEventcurrently hardcodeslaunch_typeto the touchpad value; if split-screen can be triggered by multiple gesture sources in future, you might want to pass the launch type as a parameter (similar tologMultiTaskViewEvent) to keep the logging format consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `packageVersion` and `packageNameByFile` helpers invoke `dpkg-query` synchronously on each call, which may be expensive along the gesture path; consider caching per-package results (similar to `kwinVersion`) or performing these lookups lazily/off the critical path to avoid blocking UI interactions.
- `logSplitScreenEvent` currently hardcodes `launch_type` to the touchpad value; if split-screen can be triggered by multiple gesture sources in future, you might want to pass the launch type as a parameter (similar to `logMultiTaskViewEvent`) to keep the logging format consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Ivy233
force-pushed
the
feat/kwin-gesture-event-log
branch
from
b93d9d1 to
f08a2ff
Compare
Ivy233
requested review from
18202781743,
BLumia,
fly602,
mhduiy,
robertkill,
wjyrich and
yixinshark
Ivy233
force-pushed
the
feat/kwin-gesture-event-log
branch
2 times, most recently
from
2dcf603 to
e56eff9
Compare
Add shared logging helpers for KWin-related gesture actions and include package/version context where available. Update finalized event identifiers for existing display and input device logs. feat(gesture): 增加 KWin 交互日志支持 为 KWin 相关手势动作增加共享日志辅助逻辑，并在可用时补充包与版本上下文。 更新显示与输入设备日志的正式事件标识。 PMS: TASK-388657
Ivy233
force-pushed
the
feat/kwin-gesture-event-log
branch
from
e56eff9 to
be5bc89
Compare
deepin pr auto review你好！我是 CodeGeeX。我已经仔细审查了你提供的 Git Diff 代码。这段代码主要涉及为手势功能添加事件日志记录（Event Log），并更新了部分常量定义。 以下是从语法逻辑、代码质量、代码性能和代码安全四个维度提出的详细改进意见： 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与建议修改示例代码整体质量很高，特别是性能优化和逻辑解耦做得很好。主要建议集中在日志调试信息的完善和常量类型的考虑上。 建议修改代码 (gesture1/event_log.go): // 建议补充日志信息以便调试
func logSplitScreenEvent() {
// ... data 结构定义不变 ...
// 建议在日志中增加版本信息，方便排查问题
if eventlog.WriteEventLog(data) {
logger.Debug("EventLog: kwin split screen, version:", getKWinVersion())
}
}
// 建议增强 packageVersion 的校验逻辑（可选）
func packageVersion(packageName string) string {
packageName = strings.TrimSpace(packageName)
// 仅允许字母、数字、点、减号和加号（符合 Debian 包名规范）
// 这比排除特定字符更安全
if packageName == "" {
return ""
}
// 简单的正则校验，确保只包含合法字符
// 注意：实际包名规则更复杂，这里仅作示例
/*
如果不想引入 regexp 包，保持原有的 ContainsAny 也是可以接受的，
但建议注释说明原因。
*/
out, err := exec.Command("dpkg-query", "-W", "-f=${Version}", packageName).Output()
if err != nil {
return ""
}
return strings.TrimSpace(string(out))
}建议修改代码 (gesture1/gesture_action.go): // ...
func (m *Manager) doTileActiveWindowLeft() error {
if err := m.wm.TileActiveWindow(0, wmTileDirectionLeft); err != nil {
return err
}
logSplitScreenEvent()
return nil
}
// ...这部分代码目前的实现已经很好，无需大改，只需注意上述细节即可。 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Add shared logging helpers for KWin-related gesture actions and include package/version context where available. Update finalized event identifiers for existing display and input device logs.
feat(gesture): 增加 KWin 交互日志支持
为 KWin 相关手势动作增加共享日志辅助逻辑，并在可用时补充包与版本上下文。
更新显示与输入设备日志的正式事件标识。
PMS: TASK-388657
Summary by Sourcery
Add event logging for KWin-related gesture interactions and refresh event identifiers for existing display and input device telemetry.
New Features:
Enhancements:
Chores: