Skip to content

fix(tui): preserve raw LF for Shift+Enter newline - #18229

Closed
morozsm wants to merge 1 commit into
NousResearch:mainfrom
morozsm:fix/tui-lf-shift-enter-newline
Closed

fix(tui): preserve raw LF for Shift+Enter newline#18229
morozsm wants to merge 1 commit into
NousResearch:mainfrom
morozsm:fix/tui-lf-shift-enter-newline

Conversation

@morozsm

@morozsm morozsm commented May 1, 2026

Copy link
Copy Markdown

Summary

Fixes TUI multiline input for terminal stacks that distinguish plain Enter and Shift+Enter by raw CR/LF bytes.

The TUI TextInput already checks event.keypress.raw === '\n' to insert a newline, but hermes-ink was clearing raw for both \r and \n when normalizing them to key.return. This meant LF from Shift+Enter/Ctrl+J could no longer be distinguished from CR from Enter, so it submitted instead of inserting a newline.

This PR preserves keypress.raw for CR/LF and adds regression coverage.

Fixes #18228

Test Plan

  • npm --prefix ui-tui test -- parse-keypress.test.ts --run
  • npm --prefix ui-tui run build
  • Manual verification: on macOS Warp/tmux, Enter sends CR (13) and Shift+Enter sends LF (10); after the fix, Shift+Enter inserts a newline and Enter submits.

@ddiall

ddiall commented May 8, 2026

Copy link
Copy Markdown

Tested on WSL2 + Windows Terminal. Ctrl+Enter works correctly for inserting newlines after this patch. Note for Windows Terminal users: Shift+Enter sends CR (same as Enter) rather than LF, so Shift+Enter still submits -- Ctrl+Enter is the reliable shortcut on that platform until Kitty keyboard protocol is fully negotiated.

Would be worth updating the PR description or a follow-up comment to note the platform difference in case other WSL users find this PR.

@paulocarmino

Copy link
Copy Markdown

I ended up creating a skill to auto-reapply this patch after every Hermes update, been three weeks and this is still open.
It works perfectly on Warp/macOS. Would love to see it merged so I can drop the local workaround.

@dso2ng

dso2ng commented May 20, 2026

Copy link
Copy Markdown
Contributor

I built a small follow-up branch on top of this PR that keeps the CR/LF preservation from #18229 but gates the raw-LF newline fallback so it doesn't turn every bare LF into multiline intent in thin/local PTY environments where plain Enter may also arrive as LF.

Branch / compare:
morozsm:fix/tui-lf-shift-enter-newline...dso2ng:fix/tui-modified-enter-intent

What it adds:

  • keeps explicit modified Return (Shift+Enter, Ctrl+Enter, macOS action-mod Return) as newline intent
  • treats raw LF (\n) as newline intent only for environments where plain Enter is expected to remain distinct from LF, e.g. SSH/WSL/Windows Terminal and known terminal programs like WezTerm/Warp/Ghostty/Kitty
  • keeps bare LF as submit intent for generic local POSIX/thin PTY fallback cases, avoiding the "Enter becomes dead / never submits" risk mentioned in the classic CLI handling
  • adds textInputModifiedEnter.test.ts coverage for explicit modified Return, raw LF allowlisted terminals, SSH/WSL/WT, and the generic thin-PTY fallback

Local verification on the branch:

  • npm --prefix ui-tui test -- --run packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputModifiedEnter.test.ts -> 23 passed
  • npm --prefix ui-tui run type-check -> passed
  • npm --prefix ui-tui run build -> passed
  • non-ASCII added-lines guard -> non_ascii_added_lines=0

Happy to turn this into a patch against this PR if that shape is useful; I avoided opening a competing PR since this PR already owns the root cause.

@neo4j-dan

Copy link
Copy Markdown

Do this in iTerm2:

1. iTerm2 → Settings (Cmd+,) → Keys tab → Key Bindings (or do it per-profile under Profiles → Keys → Key Mappings if you only want it for one profile).
2. Click the + to add a new mapping.
3. Click the "Keyboard Shortcut" field, then physically press Shift+Return. It should register as "⇧↩" / Shift+Return.
4. Set Action to: Send Escape Sequence.
5. In the Esc+ value field, type exactly: [13;2u
   (iTerm prepends the ESC byte automatically, so this yields \x1b[13;2u — precisely what install_shift_enter_alias() maps to a newline.)
6. Click OK / Add.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused TUI investigation. This is an automated hermes-sweeper review; current main already provides the requested behavior through a more targeted implementation.

  • cf8862cfa316626ab4e673b9e04e0105a937f9bd added LF/Ctrl+J preservation for supported terminal environments and is in current main.
  • ui-tui/src/components/textInput.tsx:993-997 retains the LF distinction via keypress.sequence and inserts a newline when the runtime is eligible.
  • ui-tui/src/components/textInput.tsx:128-150 gates that fallback for Windows Terminal, SSH, Ghostty, and WSL, avoiding the generic bare-LF submit regression noted in the follow-up discussion.
  • ui-tui/src/__tests__/textInputPassThrough.test.ts:8-21 covers both the preserved-newline and generic-local-submit contracts.

The PR's raw-field preservation is therefore superseded by the current sequence-based implementation.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TUI Shift+Enter submits instead of newline when terminal sends LF

7 participants