Conversation
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
|
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRenames C mouse API functions to drop "Err" suffixes, extends double-click to accept a count, and updates Go APIs: introduces ClickV1 and MultiClick, replaces ClickE with a new Click that accepts count and returns error, and changes error formatting to include count. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Go as Go (robotgo.go)
participant C as C shim (mouse/mouse_c.h)
participant OS as OS (macOS / Other)
Client->>Go: Call Click/ClickV1/MultiClick(args...)
Go->>Go: validate args, determine button & count
Go->>C: C.toggleMouse(down, button)
C->>OS: perform mouse down
OS-->>C: result code
C-->>Go: return code
Go->>Go: optional delay based on count
Go->>C: C.toggleMouse(up, button)
C->>OS: perform mouse up
OS-->>C: result code
C-->>Go: return code
Go-->>Client: return error if any (includes count in message)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the mouse clicking API by consolidating error handling, adding a new MultiClick function, and simplifying the C code. The main Click function now returns errors (replacing ClickE), the old version is preserved as ClickV1, and several C wrapper functions have been removed in favor of direct error-returning implementations.
Changes:
- Renamed
ClicktoClickV1andClickEtoClickto make error-returning the default behavior - Added
MultiClickfunction for performing multiple consecutive clicks - Updated C functions to remove non-error-returning wrappers (toggleMouse, clickMouse, doubleClick now return errors directly)
- Added error handling to the
Togglefunction - Changed error messages from "double=bool" format to "count=int" format
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| robotgo.go | Refactors Click API to return errors by default, adds MultiClick function, updates Toggle with error handling |
| mouse/mouse_c.h | Removes wrapper functions and consolidates error handling in C code, includes minor formatting cleanup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
robotgo.go
Outdated
| defer MilliSleep(MouseSleep) | ||
|
|
||
| for i := 0; i < count; i++ { | ||
| if err := Click(btn, false, count); err != nil { |
There was a problem hiding this comment.
The MultiClick function passes the total count value as the third argument to Click, but this is incorrect. The Click function's signature is Click(button string, double bool, ...), so the third argument would be interpreted as something else (and based on line 724-726, it's trying to access args[3] for count). This will result in incorrect behavior. The count parameter in Click appears to be meant for error reporting only, not as a loop counter. Remove the count parameter from this Click call.
| if err := Click(btn, false, count); err != nil { | |
| if err := Click(btn, false); err != nil { |
robotgo.go
Outdated
| C.toggleMouse(C.bool(down), button) | ||
| code := C.toggleMouse(C.bool(down), button) | ||
| if code != 0 { | ||
| return formatClickError(int(code), button, "down", 1) |
There was a problem hiding this comment.
The error stage is hardcoded to "down" even when the down parameter is false (indicating an "up" action). This will produce misleading error messages when a mouse up operation fails. The stage should be set to "up" when down is false.
| return formatClickError(int(code), button, "down", 1) | |
| stage := "down" | |
| if !down { | |
| stage = "up" | |
| } | |
| return formatClickError(int(code), button, stage, 1) |
| // Click click the mouse button and return error | ||
| // | ||
| // robotgo.ClickE(button string, double bool) | ||
| // robotgo.Click(button string, double bool) | ||
| // | ||
| // Examples: | ||
| // | ||
| // err := robotgo.ClickE() // default is left button | ||
| // err := robotgo.ClickE("right") | ||
| func ClickE(args ...interface{}) error { | ||
| // err := robotgo.Click() // default is left button | ||
| // err := robotgo.Click("right") | ||
| func Click(args ...interface{}) error { |
There was a problem hiding this comment.
The Click function signature in the documentation comment still references the old API with (button string, double bool), but the actual implementation now supports an additional count parameter at args[3]. The documentation should be updated to reflect the new signature or the count parameter implementation should be reconsidered since it appears incomplete.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
robotgo.go (2)
702-742: Fix count indexing + type validation to avoid silent 0 and panics.
countis read fromargs[3], so a 3‑arg call never sets it, and the unchecked cast can panic. Also, defaulting to 0 makes error messages misleading for single clicks.🛠️ Suggested fix
- count int + count = 1 ) @@ - if len(args) > 3 { - count = args[3].(int) - } + if len(args) > 2 { + c, ok := args[2].(int) + if !ok { + return errors.New("third argument must be click count (int)") + } + count = c + }
833-846: Use the correct stage label for Toggle errors.
On an “up” toggle, the error message still reportsdown, which is misleading.🛠️ Suggested fix
- code := C.toggleMouse(C.bool(down), button) + stage := "down" + if !down { + stage = "up" + } + code := C.toggleMouse(C.bool(down), button) if code != 0 { - return formatClickError(int(code), button, "down", 1) + return formatClickError(int(code), button, stage, 1) }
🤖 Fix all issues with AI agents
In `@robotgo.go`:
- Around line 745-759: MultiClick is passing the result of CheckMouse (a
C.MMMouseButton) into Click, causing a type mismatch; change the call to use the
original button string instead (keep calling CheckMouse(button) to
validate/prepare the button but pass button to Click), i.e. update the Click
invocation inside MultiClick to pass the string first argument; refer to
function names MultiClick, CheckMouse and Click when locating and fixing the
call.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mouse/mouse_c.h (1)
203-236:countparameter ignored on non-macOS platforms.The
countparameter is only used on macOS (line 219) to setkCGMouseEventClickState. On Windows and Linux (lines 229-236), the function always performs exactly 2 clicks regardless of thecountvalue passed. This breaks theMultiClickfunctionality on non-macOS platforms.🐛 Proposed fix to honor count on all platforms
`#else` /* Double click for everything else. */ - int err = clickMouse(button); - if (err != 0) { - return err; + for (int i = 0; i < count; i++) { + int err = clickMouse(button); + if (err != 0) { + return err; + } + if (i < count - 1) { + microsleep(200); + } } - microsleep(200); - return clickMouse(button); + return 0; `#endif`
🤖 Fix all issues with AI agents
In `@robotgo.go`:
- Around line 724-726: The code unsafely reads count via args[3] without a type
check and also skips args[2], which is confusing and can panic; update the
handler that sets count to (1) use the intended parameter index (either args[2]
if count is the 3rd arg or keep args[3] but then document why) and (2) perform a
safe type assertion (use a type switch or v, ok := args[index].(int) and
fallback/default if !ok) before assigning to the count variable; also update or
add documentation near the existing comment block that describes the parameter
position and expected type so callers aren’t surprised.
♻️ Duplicate comments (2)
robotgo.go (2)
760-764: MultiClick passes wrong type to Click (still broken).
Click(btn, false, count)passesbtnwhich isC.MMMouseButton, butClickexpects a string as the first argument. This will always return "first argument must be a button string".🐛 Proposed fix
for i := 0; i < count; i++ { - if err := Click(btn, false, count); err != nil { + if err := Click(button, false); err != nil { return err } }
851-857: Error stage hardcoded to "down" even for mouse-up actions.When
downisfalse, the error message will incorrectly report "click down failed" instead of "click up failed". This was flagged in a previous review.🐛 Proposed fix
+ stage := "down" + if !down { + stage = "up" + } code := C.toggleMouse(C.bool(down), button) if len(key) > 2 { MilliSleep(MouseSleep) } - return formatClickError(int(code), button, "down", 1) + return formatClickError(int(code), button, stage, 1)
🧹 Nitpick comments (1)
robotgo.go (1)
755-758: Consider returningnilexplicitly on success for clarity.The current code works because
formatClickErrorreturnsnilwhencode == 0, but the intent would be clearer with an explicit success check.♻️ Optional refactor for clarity
if runtime.GOOS == "darwin" && len(click) <= 0 { code := C.doubleClick(btn, C.int(count)) + if code == 0 { + return nil + } return formatClickError(int(code), btn, "down", count) }
2 participants
Please provide Issues links to:
Provide test code:
Description
...
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.