feat: support Shift+Enter for composer newlines#462
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Shift+Enter as an additional newline-insertion shortcut in the chat composer while keeping the existing Enter-to-submit behavior intact.
Changes:
- Treat Shift+Enter the same as Alt+Enter to insert a newline in the composer.
- Update the keyboard shortcut help text to mention the new binding.
- Fix/update composer tests to cover Shift+Enter explicitly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/tui/model.go | Routes Shift-modified Enter through the composer key handler before submit. |
| internal/tui/composer.go | Extends newline insertion logic to accept Shift+Enter in addition to Alt+Enter. |
| internal/tui/keybinding_help.go | Updates help overlay text to include Shift+Enter for newline insertion. |
| internal/tui/composer_test.go | Updates tests to validate Shift+Enter inserts a newline without submitting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bindings: []keybinding{ | ||
| {"Enter", "send the message"}, | ||
| {"Alt+Enter", "insert a newline (multi-line compose)"}, | ||
| {"Shift+Enter / Alt+Enter", "insert a newline (multi-line compose)"}, |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR extends composer Enter handling so Shift+Enter inserts a newline like Alt+Enter, updates the keybinding help text, and corrects tests to cover Shift+Enter and Ctrl+J separately. ChangesShift+Enter multi-line compose
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/tui/composer.go (1)
309-309: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared
keyAlt(msg) || keyShift(msg)predicate.This exact condition is now duplicated verbatim in
internal/tui/model.go(Line 1239) and here. If one call site is updated later (e.g., to add another modifier or gate a suggestion path) without updating the other, the model-level routing gate and the composer's own newline-insertion condition could silently diverge — one path swallowing Enter for a modifier the other no longer honors.♻️ Suggested consolidation
+func isNewlineEnterKey(msg tea.KeyMsg) bool { + return keyIs(msg, tea.KeyEnter) && (keyAlt(msg) || keyShift(msg)) +}Then use
isNewlineEnterKey(msg)in bothmodel.go'supdateModelandcomposer.go'sapplyComposerKey.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/composer.go` at line 309, The Enter-with-modifier check is duplicated between applyComposerKey in composer.go and updateModel in model.go, so extract the shared keyAlt(msg) || keyShift(msg) predicate into a single helper such as isNewlineEnterKey(msg). Update both call sites to use that helper so the model-level routing gate and the composer newline insertion logic stay in sync if modifier handling changes later.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/tui/composer.go`:
- Line 309: The Enter-with-modifier check is duplicated between applyComposerKey
in composer.go and updateModel in model.go, so extract the shared keyAlt(msg) ||
keyShift(msg) predicate into a single helper such as isNewlineEnterKey(msg).
Update both call sites to use that helper so the model-level routing gate and
the composer newline insertion logic stay in sync if modifier handling changes
later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 223df9bf-d0ac-488e-9c94-503ca1c5bb24
📒 Files selected for processing (4)
internal/tui/composer.gointernal/tui/composer_test.gointernal/tui/keybinding_help.gointernal/tui/model.go
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approve — near-zero-cost UX win, purely additive
Worth it: yes. Shift+Enter for a newline (Enter=submit) is the near-universal convention in modern chat UIs. This adds it as an additional binding at essentially zero maintenance cost (+5/−4). Thanks @pengdst.
The change is tight and correct: plain Enter still falls through to handleSubmit() (submit unchanged), only Enter+Shift / Enter+Alt route to newline insertion, and the existing Alt+Enter / Ctrl+J newline shortcuts are preserved. Nice catch that the old "shift enter" test case was mislabeled (it actually sent Ctrl+J) — the PR now sends a real Shift+Enter and keeps ctrl j as its own case, a genuine coverage improvement.
Verified on Windows (worktree): go build ./internal/tui/... OK; the composer/Enter tests pass. No test hardcodes the changed help string, so no #414-class break.
Non-blocking notes:
- Terminal support (minor): Shift+Enter only produces a distinct key event under the kitty keyboard protocol / xterm
modifyOtherKeys. Terminals that don't report modifiers on Enter (default macOS Terminal.app, some basic emulators) will deliver a bare Enter and thus submit — graceful degradation (Alt+Enter/Ctrl+J remain), but a "(where your terminal supports it)" hint in the help text would set expectations. - The
keyAlt || keyShiftEnter predicate is now duplicated inmodel.goandcomposer.go; anisNewlineEnterKey(msg)helper would keep them in sync.
Overlap with #417 (configurable keybindings): that PR reworks the same keybinding_help.go Chat line and model.go Enter block, so a merge conflict is likely — whichever lands second rebases, and if #417 lands first this binding should move into its new registry. Trivial either way.
CI: only CodeRabbit ran (fork PR, Go workflows are maintainer-approval-gated — not "passing"); I validated build + tests locally on Windows. Approving.
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
VERDICT: approve
REGRESSION RISK: low
The change is purely additive: Shift+Enter is now treated the same as Alt+Enter for inserting a newline in the composer. Enter still submits, Alt+Enter and Ctrl+J still insert newlines. The guard at model.go:1239 (keyAlt(msg) || keyShift(msg)) prevents the Shift+Enter from reaching the submit path, matching the existing Alt+Enter behavior. No existing code path or keybinding is removed or altered.
BUILD / TEST
go build ./...— passgo vet ./internal/tui/...— passgofmt -lon all four changed files — cleango test ./internal/tui/... -run TestModifiedEnterInsertsNewlineWithoutSubmitting— all three subtests pass (alt_enter, shift_enter, ctrl_j)- CI all green (macOS, Ubuntu, Windows, Smoke, Security, Zero Review)
CONTRIBUTING
No linked issue. The author is a community contributor (pengdst). Per CONTRIBUTING.md, community PRs require an approved parent issue with the issue-approved label. No such issue is linked. This is a process violation, though the change itself is trivial and safe. Flagging for maintainer awareness.
FINDINGS
- Nit —
internal/tui/keybinding_help.go:33— Ctrl+J not listed in help text. The help overlay now shows "Shift+Enter / Alt+Enter" for newline insertion, but Ctrl+J is also a supported newline shortcut (handled inapplyComposerKey). Including it would keep the help overlay consistent with the actual bindings. Noted by Copilot as well.
No other findings.
EXISTING REVIEWS
- Vasanthdev2004 (APPROVED): Assessed the change as a near-zero-cost UX win, purely additive. Valid assessment on the current head. Nothing missed.
- coderabbitai (APPROVED + 1 nitpick): Suggested the same Ctrl+J help-text gap. Valid, trivial. Nothing missed.
- copilot-pull-request-reviewer (COMMENTED): Also flagged the Ctrl+J help-text omission. Valid. Nothing missed.
All reviews are on the current head (4f09f30). No stale reviews.
BOTTOM LINE
A safe, additive keybinding change with no regression risk; the missing issue link is a process gap for the maintainer to decide on.
|
hello @pengdst please rebase to main and kindly fix conflicts |
pengdst
force-pushed
the
add-shift-enter-newline
branch
from
4f09f30 to
5f20121
Compare
okay, already force pushed to resolve the conflict @kevincodex1 |
Summary
Adds Shift+Enter as an additional composer shortcut for inserting a newline.
What changed
Tests
Summary by CodeRabbit