Add MultiClickE for triple-click and multi-click support #753
Conversation
📝 WalkthroughWalkthroughMouse control API refactored to return error codes, replacing individual click/toggle functions with unified error-aware handlers. Go wrappers updated with new public error-returning APIs and internal calls adapted to use new C functions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClickE
participant formatClickError
participant multiClickErr as C.multiClickErr
participant OS as OS Layer
User->>ClickE: ClickE(args...)
ClickE->>ClickE: Parse & validate arguments
alt Valid single/double click
ClickE->>multiClickErr: multiClickErr(button, clickCount)
multiClickErr->>OS: Perform multi-click sequence
OS-->>multiClickErr: Error code (0=success)
multiClickErr-->>ClickE: Return error code
ClickE->>formatClickError: formatClickError(code, button, clickCount)
formatClickError-->>ClickE: Formatted error or nil
else Invalid arguments
ClickE->>formatClickError: formatClickError(-1, ...)
formatClickError-->>ClickE: Validation error
end
ClickE-->>User: error (nil or detailed message)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
robotgo.go (1)
837-843: Toggle ignores error fromtoggleMouseErr.The
Togglefunction now callsC.toggleMouseErrbut discards its return value, always returningnil. This defeats the purpose of the error-returning C function and violates user expectations sinceTogglehas anerrorreturn type.🔎 Proposed fix
func Toggle(key ...interface{}) error { var button C.MMMouseButton = C.LEFT_BUTTON if len(key) > 0 { button = CheckMouse(key[0].(string)) } down := true if len(key) > 1 && key[1].(string) == "up" { down = false } - C.toggleMouseErr(C.bool(down), button) + if code := C.toggleMouseErr(C.bool(down), button); code != 0 { + btnName := MouseButtonString(button) + return fmt.Errorf("toggle failed (%s, down=%v): code=%d", btnName, down, int(code)) + } if len(key) > 2 { MilliSleep(MouseSleep) } return nil }
🧹 Nitpick comments (2)
mouse/mouse_c.h (1)
199-202: Consider returning an error for invalidclickCount.Returning
0forclickCount < 1silently succeeds without performing any action. This could mask programming errors where the caller accidentally passes zero or a negative value. Consider returning an error code instead to make such issues visible.🔎 Proposed change
int multiClickErr(MMMouseButton button, int clickCount){ if (clickCount < 1) { - return 0; + return -1; // Invalid argument }robotgo.go (1)
748-760: Minor:kCGErrorSuccessentry is unreachable.Code
0maps to"kCGErrorSuccess", butformatClickErroris only called whencode != 0. Consider removing the entry or keeping it for documentation purposes.🔎 Proposed change
case "darwin": cgErrors := map[int]string{ - 0: "kCGErrorSuccess",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mouse/mouse_c.hrobotgo.go
🧰 Additional context used
🧬 Code graph analysis (2)
mouse/mouse_c.h (3)
base/xdisplay_c.h (1)
XGetMainDisplay(18-48)robotgo_win.go (1)
SendInput(39-41)mouse/mouse.h (1)
LEFT_BUTTON(12-20)
robotgo.go (1)
keycode.go (4)
WheelDown(28-28)WheelUp(29-29)WheelLeft(30-30)WheelRight(31-31)
🔇 Additional comments (6)
mouse/mouse_c.h (2)
159-196: LGTM!The error-returning
toggleMouseErrimplementation correctly handles error cases across all platforms:
- macOS properly checks for NULL event creation and releases resources before returning
- X11 correctly interprets the
Statusreturn value fromXTestFakeButtonEvent- Windows uses
GetLastError()for detailed error information
227-244: Sequential click approach for X11/Windows is reasonable.The 200ms inter-click delay should work for most system configurations. Note that this simulates multi-clicks through rapid sequential clicks rather than using a native multi-click event (unlike macOS's
kCGMouseEventClickState). Applications that rely on the OS to track click counts may behave differently.If users report inconsistent multi-click behavior on specific platforms, consider making the inter-click delay configurable or adjusting based on system settings.
robotgo.go (4)
510-526: LGTM!Clean reverse mapping of
CheckMousewith a sensible fallback for unknown button types.
685-689: LGTM!The
Clickfunction now routes throughmultiClickErr, maintaining backward compatibility by ignoring the return value. Users who need error handling can use the newClickEvariant.
724-724: Verify:defer MilliSleepexecutes even on failure.Using
deferhere means the sleep occurs regardless of whether the click succeeds or fails. This differs from the originalClickbehavior whereMilliSleepis called at the end unconditionally, so it's consistent—but worth confirming this is intended for error paths.
777-793: LGTM!Clean implementation that's consistent with
ClickE. The public API is straightforward:MultiClickE(button string, clickCount int) error.
1 participant
Please provide Issues links to:
Provide test code:
Description
This PR adds support for multi-click (triple-click, quadruple-click, etc.) and refactors the mouse click implementation to reduce code redundancy.
Why multi-click is needed
Changes
New API:
MultiClickE(button string, clickCount int) error- Perform any number of clicks with error handlingC layer refactoring:
multiClickErr(button, clickCount)functionkCGMouseEventClickStateAPI for accurate click counttoggleMouse,clickMouse,doubleClick,clickMouseErr,doubleClickErr)doubleClickbutton parameter bug (was hardcoded tokCGMouseButtonLeft)Code reduction:
Backward Compatibility
All existing APIs (
Click,ClickE,Toggle, etc.) remain unchanged.Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.