Skip to content

fix: ctrl-J is reported as KeyCR/KeyEnter on legacy terminals (fixes …#915

Merged
gdamore merged 1 commit intomainfrom
gdamore/fix-ctrl-j
Dec 20, 2025
Merged

fix: ctrl-J is reported as KeyCR/KeyEnter on legacy terminals (fixes …#915
gdamore merged 1 commit intomainfrom
gdamore/fix-ctrl-j

Conversation

@gdamore
Copy link
Owner

@gdamore gdamore commented Dec 20, 2025 •
edited by coderabbitai bot
Loading

…#911)

This was a regression relative to 2.9.

Summary by CodeRabbit

  • Bug Fixes

    • Newline handling adjusted: carriage return now maps to Enter; line feed is treated as a regular character. Control key sequences are more accurately distinguished in input mapping.

  • Tests

    • Updated expectations to match the revised input mapping and control-key behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 20, 2025 •
edited
Loading

Walkthrough

Input handling now treats \r (carriage return) as Enter while \n (newline) is no longer mapped to Enter and is emitted as a regular rune; tests were updated to expect Ctrl+J as KeyCtrlJ with ModCtrl instead of KeyEnter.

Changes

Cohort / File(s) Change Summary
Input Key Mapping
input.go		 Removed \n from the Enter mapping in the initial state; \r remains mapped to Enter. Newline (\n) now falls through to default handling and is emitted as a KeyRune when applicable.
Control Key Tests
input_test.go		 Updated test expectations for control-key mappings: Ctrl+J is now expected as KeyCtrlJ with ModCtrl rather than KeyEnter with ModNone. Adjusted tests including Enter_LF, NL, and related control-sequence cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify newline vs. carriage return handling across input parsing paths
  • Confirm updated tests cover all boundary/control-sequence cases impacted by the change

Poem

🐰 I nibble bytes where newlines play,
Carriage return now leads the way.
Ctrl+J stands proud, a proper key,
Soft hops of input, tidy and free. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing Ctrl-J being incorrectly reported as KeyCR/KeyEnter on legacy terminals, which directly aligns with the code changes narrowing the Enter key mapping and updating test expectations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch gdamore/fix-ctrl-j
📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cb42b9 and 76abaa6.

📒 Files selected for processing (2)
  • input.go (1 hunks)
  • input_test.go (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • input_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
input.go (1)
wscreen.go (1)
  • t (347-382)
🔇 Additional comments (1)
input.go (1)

429-430: LGTM! Clean fix for the Ctrl-J regression.

The change correctly distinguishes carriage return (\r, the actual Enter key) from line feed (\n, Ctrl-J). By removing \n from this case, Ctrl-J now falls through to the default control key handler (lines 432-436), which properly emits it as KeyRune with "J" and ModCtrl.

This is a minimal, targeted fix that restores correct legacy terminal behavior.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Dec 20, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 19.74%. Comparing base (e90d698) to head (76abaa6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #915   +/-   ##
=======================================
  Coverage   19.74%   19.74%           
=======================================
  Files          39       39           
  Lines        3404     3404           
=======================================
  Hits          672      672           
  Misses       2681     2681           
  Partials       51       51

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai bot reviewed Dec 20, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
input_test.go (1)

238-238: Consider renaming the test to reflect the corrected behavior.

The test name Enter_LF is misleading since LF (line feed) no longer produces KeyEnter. It now correctly produces KeyCtrlJ with ModCtrl. A name like LF_CtrlJ would be clearer.

🔎 Suggested fix 
-		{"Enter_LF", '\n', KeyCtrlJ, ModCtrl},
+		{"LF_CtrlJ", '\n', KeyCtrlJ, ModCtrl},
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e90d698 and 0cb42b9.

📒 Files selected for processing (2)
  • input.go (1 hunks)
  • input_test.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
input_test.go (1)
key.go (2)
  • KeyCtrlJ (449-449)
  • ModCtrl (318-318)
🔇 Additional comments (2)
input.go (1)

429-430: LGTM - CR/LF differentiation is now correct.

The change correctly restores the distinction between carriage return (CR, \r) and line feed (LF, \n). Only CR now maps to KeyEnter, while LF falls through to the default control character handling, which will transform it to KeyCtrlJ with ModCtrl. This fixes the regression and aligns with traditional terminal behavior.

input_test.go (1)

534-534: LGTM - Test correctly validates the fix.

The test expectation for newline now correctly expects KeyCtrlJ with ModCtrl, which aligns with the fix that differentiates LF from CR.

@gdamore gdamore merged commit da9945c into main Dec 20, 2025
15 checks passed
@gdamore gdamore deleted the gdamore/fix-ctrl-j branch December 20, 2025 02:00
@coderabbitai coderabbitai bot mentioned this pull request Jan 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gdamore