-
Notifications
You must be signed in to change notification settings - Fork 106
fix: 解决触控屏拖拽窗口移动异常的问题 #994
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts xdotool mouse down/up command invocation for touch-screen window dragging, correcting both the command string and how it is executed. Sequence diagram for updated touch drag mouse down/up handlingsequenceDiagram
actor User
participant Touchscreen as Touchscreen
participant Manager as Manager
participant Exec as os_exec
participant Xdotool as xdotool
participant WM as WindowManager
User->>Touchscreen: Drag window gesture
Touchscreen->>Manager: gestureEvent
Manager->>Exec: exec.Command("xdotool mousedown 3")
Exec->>Xdotool: run mousedown 3
Xdotool->>WM: start moving window
User-->>Touchscreen: Release finger
Touchscreen-->>Manager: gestureEndEvent
Manager->>Exec: exec.Command("xdotool mouseup 3")
Exec->>Xdotool: run mouseup 3
Xdotool->>WM: stop moving window
Class diagram for Manager gesture mouse actionsclassDiagram
class Manager {
+doToggleLaunchpad() error
+doXdotoolsMouseDown() error
+doXdotoolsMouseUp() error
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- When switching from
exec.Command("/bin/sh", "-c", cmd)to direct execution,exec.Command(cmd)will treat the entire string as the binary name; instead, split the command and arguments, e.g.,exec.Command("xdotool", "mousedown", "3")and similarly formouseup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When switching from `exec.Command("/bin/sh", "-c", cmd)` to direct execution, `exec.Command(cmd)` will treat the entire string as the binary name; instead, split the command and arguments, e.g., `exec.Command("xdotool", "mousedown", "3")` and similarly for `mouseup`.
## Individual Comments
### Comment 1
<location> `gesture1/gesture_action.go:169` </location>
<code_context>
func (m *Manager) doXdotoolsMouseDown() error {
cmd := "xdotool mousedown 3"
- out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput()
+ out, err := exec.Command(cmd).CombinedOutput()
if err != nil {
return fmt.Errorf("%s", string(out))
</code_context>
<issue_to_address>
**issue (bug_risk):** exec.Command is given a single string with spaces, which will be treated as the binary name rather than command + args
With `cmd := "xdotool mousedown 3"`, `exec.Command(cmd)` will try to execute a binary literally named `"xdotool mousedown 3"`. To mimic the previous behavior without the shell, pass args separately, e.g. `exec.Command("xdotool", "mousedown", "3").CombinedOutput()`. Do the same for the `mouseup` call.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
处理拖拽窗口的命令参数不正确,导致释放鼠标事件不对. Log: 修改触控屏鼠标参数 PMS: BUG-304177 311961 321777 346073 Influence: 触控屏拖拽窗口
deepin pr auto review这段代码的修改主要涉及对 1. 代码审查总结修改内容:将 总体评价:这是一个非常好的改进,显著提升了代码的安全性和健壮性。 2. 详细分析A. 安全性 —— 显著提升
B. 代码质量与可读性 —— 提升
C. 性能 —— 轻微提升
D. 语法逻辑 —— 修正了一个潜在的 Bug
3. 改进建议虽然当前的修改已经很好,但在错误处理方面还有优化空间: 当前错误处理: if err != nil {
return fmt.Errorf("%s", string(out))
}建议改进: if err != nil {
// 使用 fmt.Errorf 的 %w 动词包装错误,便于调用方使用 errors.Is/As 进行判断
return fmt.Errorf("xdotool failed: %w, output: %s", err, string(out))
}4. 结论这次修改非常正确,主要收益在于:
建议采纳该修改,并参考上述关于错误处理的建议进一步完善代码。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, fly602, justforlxz 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 |
处理拖拽窗口的命令参数不正确,导致释放鼠标事件不对.
Log: 修改触控屏鼠标参数
PMS: BUG-304177 311961 321777 346073
Influence: 触控屏拖拽窗口
Summary by Sourcery
Bug Fixes: