Skip to content

fix: detect Shift+Enter on macOS terminals without Kitty protocol support#2969

Open
gtardif wants to merge 1 commit into
docker:mainfrom
gtardif:fix_shift_enter_osx_terminal
Open

fix: detect Shift+Enter on macOS terminals without Kitty protocol support#2969
gtardif wants to merge 1 commit into
docker:mainfrom
gtardif:fix_shift_enter_osx_terminal

Conversation

@gtardif
Copy link
Copy Markdown
Contributor

@gtardif gtardif commented Jun 2, 2026

macOS Terminal.app (and other legacy terminals) don't support the Kitty keyboard protocol, so Shift+Enter sends the same byte as plain Enter. This makes it impossible to distinguish the two at the terminal protocol level.

Use the macOS CoreGraphics API (CGEventSourceFlagsState) to query the system-level keyboard modifier state when a plain Enter key event is received. If Shift is physically held, treat it as a newline insertion instead of submitting the message.

This approach works universally across all macOS terminal emulators without requiring any terminal configuration. For terminals that do support the Kitty protocol (VSCode, iTerm2 3.5+, Ghostty), the existing shift+enter detection continues to work via the protocol, and the CoreGraphics fallback is never reached.

…port

macOS Terminal.app (and other legacy terminals) don't support the Kitty
keyboard protocol, so Shift+Enter sends the same byte as plain Enter.
This makes it impossible to distinguish the two at the terminal protocol
level.

Use the macOS CoreGraphics API (CGEventSourceFlagsState) to query the
system-level keyboard modifier state when a plain Enter key event is
received. If Shift is physically held, treat it as a newline insertion
instead of submitting the message.

This approach works universally across all macOS terminal emulators
without requiring any terminal configuration. For terminals that do
support the Kitty protocol (VSCode, iTerm2 3.5+, Ghostty), the existing
shift+enter detection continues to work via the protocol, and the
CoreGraphics fallback is never reached.
@gtardif gtardif requested a review from a team as a code owner June 2, 2026 12:30
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Comment thread pkg/tui/components/editor/editor.go
Comment thread pkg/tui/internal/termfeatures/modifiers_darwin.go
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

This PR introduces macOS CoreGraphics-based Shift key detection to handle Shift+Enter in terminals that don't support the Kitty keyboard protocol. The build tag split (darwin / !darwin) is correct and exhaustive. The CGo constants (kCGEventSourceStateCombinedSessionState, kCGEventFlagMaskShift) are appropriate. Kitty-capable terminals (iTerm2, Ghostty, VSCode) are not affected since they send shift+enter as a distinct key event.

Two LIKELY issues found in the added code — see inline comments.

Comment thread pkg/tui/components/editor/editor.go
Comment thread pkg/tui/internal/termfeatures/keyboard.go
@gtardif gtardif marked this pull request as draft June 2, 2026 13:48
@aheritier aheritier added area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix: commit prefix) labels Jun 2, 2026
@dgageot dgageot marked this pull request as ready for review June 3, 2026 15:38
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

3 finding(s): 1 HIGH (LIKELY), 1 MEDIUM (LIKELY), 1 LOW (LIKELY). See inline comments for details.

@@ -0,0 +1,23 @@
//go:build darwin
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] CGo dependency breaks darwin cross-compilation from Linux CI

modifiers_darwin.go introduces a CGo dependency (#cgo LDFLAGS: -framework CoreGraphics + import "C"), which requires a native darwin C toolchain at build time. The project's task cross command builds darwin/amd64 and darwin/arm64 targets — if that task runs on a Linux CI host (typical for cross-platform builds), the darwin CGo target will fail to compile because:

  1. A darwin cross-compiler (e.g. osxcross) is not a standard Go CI setup, and
  2. Forcing CGO_ENABLED=0 for darwin would also break the build since the import "C" cannot be excluded.

Impact: CI pipelines doing cross-compilation for darwin will fail. Docker's own multi-platform build (task cross) is likely affected.

Suggestion: Consider adding a build tag guard or a //go:build ignore variant so that the CGo implementation is only compiled natively on a darwin host, and document that darwin binaries must be built natively. Alternatively, restructure to allow CGO_ENABLED=0 fallback (where IsShiftPressed() returns false, falling back to the ctrl+j path).

// protocol, so Shift+Enter sends the same byte as Enter. Use the macOS
// CoreGraphics API to check if Shift is physically held, and treat it
// as a newline insertion.
if msg.String() == "enter" && termfeatures.IsShiftPressed() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Timing race: Shift key may be released before IsShiftPressed() is polled

CGEventSourceFlagsState is called at the moment bubbletea dispatches the key event to the Update function — not at the exact moment the physical key was pressed. If the user releases Shift before the Go event loop processes the message (possible under goroutine scheduling delays or system load), IsShiftPressed() returns false, and the Enter is treated as a plain Enter — submitting the message instead of inserting a newline.

Impact: Intermittent false negatives: the user presses Shift+Enter intending to insert a newline, but the message is submitted unexpectedly. This is a regression compared to terminals that use the Kitty protocol (where the key event carries shift modifier flags, immune to timing).

Suggestion: The PR description acknowledges this is a best-effort approach, but this limitation should be documented (e.g., a code comment or a note in the PR). Consider whether the timing window is acceptable in practice (bubbletea dispatch is typically sub-millisecond), and if so, document it explicitly to set expectations.

// CoreGraphics API to check if Shift is physically held, and treat it
// as a newline insertion.
if msg.String() == "enter" && termfeatures.IsShiftPressed() {
e.textarea.InsertString("\n")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] InsertString("\n") vs InsertNewline may behave differently

The Kitty-protocol path inserts a newline by passing the key event through e.textarea.Update(msg), which dispatches the InsertNewline key binding internally. The new macOS path directly calls e.textarea.InsertString("\n").

For the current textarea implementation these are likely equivalent, but if the bubbletea textarea widget ever adds smart-indentation, auto-pairing, or other InsertNewline-specific logic, the two paths will diverge silently.

Suggestion: If a direct InsertNewline API is available on the textarea, prefer calling it for consistency with the Kitty-protocol path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants