Skip to content

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

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.

Sign up for GitHub

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

Open
gtardif wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −4
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/tui/components/editor/editor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,16 @@ func (e *editor) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
return e, nil
}

// On macOS, terminals like Terminal.app don't support the Kitty keyboard
// 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() {
Comment thread
gtardif marked this conversation as resolved.
Copy link
Copy Markdown

@docker-agent docker-agent Jun 3, 2026

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.

e.textarea.InsertString("\n")
Comment thread
gtardif marked this conversation as resolved.
Copy link
Copy Markdown

@docker-agent docker-agent Jun 3, 2026

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.

e.refreshSuggestion()
return e, nil
}

// Let textarea process the key - it handles newlines via InsertNewline binding
prev := e.textarea.Value()
e.textarea, _ = e.textarea.Update(msg)
Expand Down
11 changes: 10 additions & 1 deletion pkg/tui/internal/termfeatures/keyboard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
package termfeatures

import "strings"
import (
"runtime"
"strings"
)

// SupportsModifiedEnter returns true for terminals that can distinguish
// Shift+Enter from Enter even when they do not report Kitty keyboard flags.
// On macOS, we use the CoreGraphics API to detect modifier key state at the
Comment thread
gtardif marked this conversation as resolved.
// system level, so all terminals effectively support this.
func SupportsModifiedEnter(getenv func(string) string) bool {
if runtime.GOOS == "darwin" {
return true
}

if getenv == nil {
return false
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/tui/internal/termfeatures/keyboard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package termfeatures

import "testing"
import (
"runtime"
"testing"
)

func TestSupportsModifiedEnter(t *testing.T) {
t.Parallel()
Expand All @@ -14,8 +17,8 @@ func TestSupportsModifiedEnter(t *testing.T) {
{name: "wezterm pane", env: map[string]string{"WEZTERM_PANE": "1"}, want: true},
{name: "wezterm socket", env: map[string]string{"WEZTERM_UNIX_SOCKET": "/tmp/wezterm.sock"}, want: true},
{name: "wezterm term", env: map[string]string{"TERM": "wezterm"}, want: true},
{name: "other terminal", env: map[string]string{"TERM_PROGRAM": "Apple_Terminal", "TERM": "xterm-256color"}, want: false},
{name: "nil getenv", env: nil, want: false},
{name: "other terminal", env: map[string]string{"TERM_PROGRAM": "Apple_Terminal", "TERM": "xterm-256color"}, want: runtime.GOOS == "darwin"},
{name: "nil getenv", env: nil, want: runtime.GOOS == "darwin"},
}

for _, tt := range tests {
Expand Down
23 changes: 23 additions & 0 deletions pkg/tui/internal/termfeatures/modifiers_darwin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build darwin
Copy link
Copy Markdown

@docker-agent docker-agent Jun 3, 2026

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).


package termfeatures

/*
#cgo LDFLAGS: -framework CoreGraphics
#include <CoreGraphics/CoreGraphics.h>

static int isShiftPressed() {
CGEventFlags flags = CGEventSourceFlagsState(kCGEventSourceStateCombinedSessionState);
Comment thread
gtardif marked this conversation as resolved.
return (flags & kCGEventFlagMaskShift) != 0;
}
*/
import "C"

// IsShiftPressed queries the macOS CoreGraphics event system to determine
// whether the Shift key is currently held down. This allows us to distinguish
// Shift+Enter from Enter in terminals that don't support the Kitty keyboard
// protocol (like macOS Terminal.app), since those terminals send the same byte
// for both key combinations.
func IsShiftPressed() bool {
return C.isShiftPressed() != 0
}
9 changes: 9 additions & 0 deletions pkg/tui/internal/termfeatures/modifiers_other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build !darwin

package termfeatures

// IsShiftPressed returns false on non-macOS platforms. The CoreGraphics-based
// modifier detection is only available on macOS.
func IsShiftPressed() bool {
return false
}
Loading