-
Notifications
You must be signed in to change notification settings - Fork 0
avoid double backspace #133
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
base: main
Are you sure you want to change the base?
Conversation
Enable external text input sources like Superwhisper (speech-to-text) and the emoji picker to work with SDL windows. SDL doesn't natively support NSTextInputClient for accessibility-based input. The solution adds a transparent NSTextView overlay that: - Acts as first responder to receive accessibility queries - Forwards keyboard events to SDL for normal input - Intercepts Cmd+V to handle paste (used by Superwhisper) - Captures insertText: for emoji picker and keyboard input - Exposes accessibility attributes so apps can discover the text field
Both emoji picker and Superwhisper simulate Cmd+V after putting text on the pasteboard. The Cmd+V interception handles both cases, so the pasteboard check on window focus was causing double-paste and also incorrectly pasting clipboard contents when switching back to the app.
Issue: Backspace deletes two characters after adding accessibility paste/input support. Solution: Route SDL and accessibility text input through a shared handler. Filter backspace control bytes (0x08/0x7f) out of text payloads to avoid duplicate deletes. Document macOS accessibility input and backspace filtering behavior.
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.
Pull request overview
This PR implements macOS accessibility support to prevent duplicate backspace events when using external text input sources like emoji pickers and speech-to-text apps. The solution adds a hidden NSTextView that exposes proper accessibility attributes, keeps SDL text input active across focus changes, and filters backspace control bytes from text payloads to ensure deletion only occurs via keyboard events.
Changes:
- Added macOS-specific accessibility text input layer using Objective-C NSTextView with Zig wrapper
- Modified text input handling to keep SDL text input active across window focus changes
- Implemented backspace control byte filtering (0x08/0x7f) in text input processing
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/sdl.zig | Added SDL text input area configuration function |
| src/platform/macos_text_input.zig | New Zig wrapper for macOS accessibility text input |
| src/platform/macos_text_input.m | New Objective-C implementation of accessible NSTextView |
| src/main.zig | Integrated accessibility input, removed focus-based text input toggling, added backspace filtering |
| src/c.zig | Added SDL3 property and text input area API bindings |
| docs/architecture.md | Documented backspace filtering behavior |
| build.zig | Added Objective-C source file compilation with ARC |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/platform/macos_text_input.zig
Outdated
|
|
||
| // Store the new text | ||
| instance.pending_text = instance.allocator.dupe(u8, text) catch { | ||
| log.err("Failed to allocate text buffer", .{}); |
Copilot
AI
Jan 16, 2026
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.
The error log message is missing format placeholders. While the current code compiles, the logging framework expects format arguments. Consider using log.err(\"Failed to allocate text buffer\", .{}); consistently with other logging calls, or add relevant error context if available.
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.
Won't fix. In Zig's scoped logging API, .{} is the standard way to pass an empty format argument struct. The format specifier is always the second parameter, and passing .{} is idiomatic for messages without dynamic content.
src/platform/macos_text_input.m
Outdated
| // after putting text on the pasteboard. SDL receives other keys through its own mechanism. | ||
| - (void)keyDown:(NSEvent*)event { | ||
| NSEventModifierFlags cmdOnly = event.modifierFlags & NSEventModifierFlagDeviceIndependentFlagsMask; | ||
| if (cmdOnly == NSEventModifierFlagCommand && event.keyCode == 9) { // Cmd+V |
Copilot
AI
Jan 16, 2026
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.
The magic number 9 for the Cmd+V keyCode should be defined as a named constant (e.g., kVirtualKeyCode_V or KEY_CODE_V) to improve code readability and maintainability.
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.
Fixed. Added kKeyCodeV constant for the macOS virtual key code.
src/platform/macos_text_input.m
Outdated
|
|
||
| // Make it nearly transparent but still functional | ||
| // Note: alpha=0 causes issues with text input | ||
| [g_textView setAlphaValue:0.01]; |
Copilot
AI
Jan 16, 2026
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.
The magic number 0.01 for alpha value should be defined as a named constant (e.g., MINIMUM_VISIBLE_ALPHA) with a comment explaining why this specific value is required instead of 0.
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.
Fixed. Added kTextViewAlpha constant with a comment explaining its purpose.
src/main.zig
Outdated
| var sent_any = false; | ||
| while (idx < text.len) : (idx += 1) { | ||
| const ch = text[idx]; | ||
| if (ch == 8 or ch == 0x7f) { |
Copilot
AI
Jan 16, 2026
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.
The magic numbers 8 and 0x7f for backspace control bytes should be defined as named constants (e.g., BACKSPACE_CTRL and DELETE_CTRL) to improve code clarity and maintainability.
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.
Fixed. Added CTRL_BACKSPACE and CTRL_DELETE named constants.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2558a64d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/platform/macos_text_input.zig
Outdated
| instance.allocator.free(old); | ||
| } | ||
|
|
||
| // Store the new text | ||
| instance.pending_text = instance.allocator.dupe(u8, text) catch { |
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.
Queue multiple accessibility text events
The accessibility callback replaces pending_text each time it fires, and pollText() only returns a single slice per frame. If macOS delivers multiple insertText:/accessibility updates before the main loop polls (common with speech-to-text or IME updates), earlier chunks are freed and lost, so users see missing characters. Consider appending to a buffer or storing a small queue so successive callbacks are preserved.
Useful? React with 👍 / 👎.
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.
Fixed. Changed pending_text from ?[]u8 to std.ArrayList(u8) to queue multiple text events instead of replacing.
- Queue multiple text events using ArrayList instead of replacing - Add named constants for control characters (CTRL_BACKSPACE, CTRL_DELETE) - Add named constants for macOS text input (kTextViewAlpha, kKeyCodeV)
Solution: