-
Notifications
You must be signed in to change notification settings - Fork 2
Improve e2e auto paste performance - #30
Conversation
|
📋 PR Summary Replaces the macOS clipboard backend: Changes
|
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.
Warning
Changes suggested — 🟡 1 warning · 1 minor point
🔍 Full review · 16 files reviewed
🔵 Minor points
Not blocking, and no threads opened for these.
log/log.go:245— The LatencyBreakdown field comments name a mechanism this PR removes: ClipSaveMs is documented as "pbpaste fork", PasteCopyMs (line 247) as "pbcopy fork", and the struct doc (line 236) as "the pbpaste fork's own duration". After this change macOS SaveCurrent/Copy go through NSPasteboard (no fork), and the Linux backend forks xclip/xsel — not pbpaste/pbcopy — so the comments describe a fork that runs on no platform. The added clipSave doc in main.go ("how long the pbpaste fork took") is stale for the same reason. Reword to name the clipboard save/copy stage generically.
Review details
- Commit: 54425b3
- Model: claude-opus-4-8
- Panel: security · correctness · robustness · design
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.
🟡 Warning — A NULL event-create result crashes the app instead of dropping the paste.
clipPaste never checks the results of CGEventCreateKeyboardEvent. When the function returns NULL (event source cannot be created under resource pressure or certain TCC/sandbox states), CGEventSetFlags(down, ...) is undefined and CFRelease(down)/CFRelease(up) on a NULL ref is a documented crash — so a failed event creation terminates the app on the paste path instead of a silently-dropped keystroke. Guard each ref (return early / skip CFRelease when NULL) as the replaced keybd_event path did internally.
No description provided.