Add toggle: Rich Input submit on Ctrl+Enter (vs Enter)#11723
Conversation
Adds an opt-in AISettings boolean `submit_on_ctrl_enter` (default false, synced globally, toml path `agents.third_party.submit_on_ctrl_enter`). When enabled and the CLI-agent Rich Input is open: Enter inserts a newline; Ctrl+Enter submits. Shell-mode `!` prefix handling is preserved on both submit paths. UI: new toggle row in `CLIAgentWidget` following the existing `ShouldRenderCLIAgentToolbar` pattern, wired through `AISettingsPageAction::ToggleSubmitRichInputOnCtrlEnter`. Refactor: extract `Input::input_ctrl_enter` to mirror the existing `input_enter` / `input_cmd_enter` shape and unify the CtrlEnter path. Tests: 6 new tests in `app/src/terminal/input_tests.rs` covering the four-cell Enter/Ctrl+Enter × setting on/off matrix, the shell-mode prefix regression branch, and the settings default + round-trip. Refs warpdotdev#11588
The keymap-context predicate for EditorAction::CtrlEnter (editor/view/mod.rs:393-399) suppresses the binding when AGENT_VIEW_ENABLED and CTRL_ENTER_ENTERS_AGENT_VIEW are both set. On Windows with AgentView FF on, the Input's keymap_context_modifier was setting CTRL_ENTER_ENTERS_AGENT_VIEW whenever the inline agent-view controller was inactive — which the CLI agent rich input does not toggle — so the keystroke was dropped before EditorEvent::CtrlEnter could fire. Add a third clause to the conditional so CTRL_ENTER_ENTERS_AGENT_VIEW is NOT set when the CLI agent rich input is open. The same handle is read one line below to set CLI_AGENT_RICH_INPUT_OPEN, so the change is exactly parallel. Tests: two new keymap-context tests in input_tests.rs covering the rich-input-open and rich-input-closed cases — would have caught the original bug before warp-oss smoke-test. Refs warpdotdev#11588 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds end-to-end coverage of the submit_on_ctrl_enter setting under crates/integration/, driving real keystrokes through the editor event dispatch pipeline rather than calling input_ctrl_enter directly. This exercises the editor's default InsertNewLineIfMultiLine behavior plus the backspace-then-submit sequence that the unit tests cannot reach. Adds 4 tests covering the 2x2 keystroke x setting-state matrix and introduces new helpers in app/src/integration_testing/input/step.rs (open_cli_agent_rich_input plus three buffer-state assertions) — no prior CLI-Agent / Rich-Input coverage existed in the integration crate. refs warpdotdev#11588
Asserts the input buffer is cleared after Ctrl+Enter submit in ctrl_enter_submits_when_submit_on_ctrl_enter_is_true (was only verifying the submitted text, not the post-submit buffer state). Adds a short comment above editor.backspace() in input.rs explaining that EditorEvent::CtrlEnter triggers the editor's default InsertNewLineIfMultiLine, which inserts a trailing newline that this backspace strips before submit so the submitted message is clean. refs warpdotdev#11588
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a synced setting for CLI-agent Rich Input to submit on Ctrl+Enter instead of Enter, wires the behavior through the input event path, exposes the toggle in Settings, and adds unit/integration test coverage.
Concerns
- The PR changes user-visible Settings UI and input behavior, but the Screenshots / Videos section only contains a placeholder upload comment rather than an attached screenshot or recording. Repository review guidance requires visual evidence for user-facing changes before merge.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…-rich-input-ctrl-enter # Conflicts: # app/src/integration_testing/input/step.rs # app/src/terminal/input_tests.rs
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a synced setting and settings toggle for CLI-agent Rich Input to submit on Ctrl+Enter instead of Enter, updates the terminal input handling, and adds unit/integration coverage for the new key matrix.
Concerns
- Ctrl+Enter submission currently relies on the editor first inserting a newline and then calling
backspace()before submit. If the user has selected text, that newline insertion replaces the selection, and the backspace only removes the temporary newline, so the selected text is silently dropped from the submitted prompt.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Oz code review caught a silent data-loss bug: with submit_on_ctrl_enter
on and text selected in the Rich Input, Ctrl+Enter dropped the selected
content from the submitted message.
Root cause: the editor's default ctrl_enter behavior is
EnterAction::InsertNewLineIfMultiLine, which calls
editor_model.insert("\n", ...) — and insert replaces the current
selection with the new content. The handler's editor.backspace() then
removed the "\n" but the selected text was already gone.
Fix: stop relying on backspace. Set the Rich Input editor's
enter_settings reactively via a new update_cli_agent_enter_settings
helper that fires on both CLI-agent input state changes and on the
SubmitRichInputOnCtrlEnter setting toggle:
- ctrl_enter is always EnterAction::Emit for Rich Input — Ctrl+Enter
goes straight to the submit path without touching the buffer.
- enter is InsertNewLineIfMultiLine when the toggle is on (so Enter
inserts a newline) and Emit otherwise (current Enter-submits behavior).
The backspace hack at input.rs:10238-10254 is removed; the handler
reduces to a single dispatch line.
Adds a regression unit test that would have failed under the old
implementation (selecting text + Ctrl+Enter must submit the full buffer,
including the selection) and an integration test exercising the same
flow through the editor event-dispatch pipeline.
refs warpdotdev#11588
|
@oz-for-oss thanks — good catch, this was a real silent data-loss bug. Fixed in 15b4fe3. Root cause (matching your trace): the editor's default Fix: stopped using the backspace pattern entirely. Added
The 15-line backspace block in Regression coverage:
🤖 Generated by Claude Code on behalf of @cbeaulieu-gt |
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
Adds a synced setting and Settings toggle to let CLI-agent Rich Input submit on Ctrl+Enter while Enter inserts newlines, with unit and integration coverage for the key matrix.
Concerns
- With the toggle enabled, the editor consumes Enter before
Input::input_entercan handle existing Rich Input inline menus, so @ context, prompt, skill, and slash menus no longer accept the highlighted item with Enter.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Oz code review caught a second bug from the prior selection fix: with submit_on_ctrl_enter on, pressing Enter no longer accepted the highlighted item in Rich Input inline menus (@-context, prompt, skill, slash command). The editor consumed Enter via InsertNewLineIfMultiLine and never emitted Event::Enter, so Input::input_enter — which contains all the menu-acceptance branches — never ran. Fix mirrors the Ctrl+Enter principle: stop having the editor consume Enter. Both enter_settings.enter and enter_settings.ctrl_enter are now always EnterAction::Emit for Rich Input (regardless of the toggle). The toggle-aware newline-insertion decision moves into input_enter itself, placed after all menu-acceptance early-returns so menus get first crack at Enter. Side effect: update_cli_agent_enter_settings no longer depends on the SubmitRichInputOnCtrlEnter setting and only fires on Rich Input open/close. The settings-changed call site is kept as a no-op with a comment, since removing it bloats the lifecycle reasoning. Adds a regression unit test that programmatically opens the slash commands menu, presses Enter with the toggle on, and asserts the buffer does NOT contain a newline (menu-acceptance branch was taken). Adds an integration test exercising the same flow through the full keystroke dispatch pipeline. refs warpdotdev#11588
|
@oz-for-oss good catch again — this was caused by my previous Ctrl+Enter fix and mirrors the same shape. Fixed in c1cd11d. Root cause (matching your trace): the prior fix set Fix (same principle as the Ctrl+Enter fix): stop having the editor consume Enter. Selection semantics are intentionally asymmetric:
Regression coverage:
7/7 unit tests pass; full verification suite green. 🤖 Generated by Claude Code on behalf of @cbeaulieu-gt |
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a synced setting and settings-page toggle for submitting CLI-agent Rich Input with Ctrl+Enter, updates the input/editor event path, and adds unit plus integration coverage for the key behavior matrix. The attached PR description includes visual evidence for the user-facing settings/behavior change.
Concerns
- The new enter-settings updater is invoked when Rich Input closes but always leaves the reused editor's Ctrl+Enter behavior set to
Emit, which regresses the normal input editor's default Ctrl+Enter newline behavior after a CLI-agent Rich Input session. - No security findings.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Oz code review caught the third bug in this series. The editor view is
shared between Rich Input (CLI agent) mode and normal terminal mode.
The update_cli_agent_enter_settings helper was setting
{enter: Emit, ctrl_enter: Emit} unconditionally on both open AND close,
so after closing a Rich Input session the normal terminal's default
Ctrl+Enter newline behavior was permanently broken.
Fix: branch on CLIAgentSessionsModel::is_input_open. When Rich Input is
open, apply {Emit, Emit} (Rich Input-specific behavior). When closed,
apply EnterSettings::default() so the normal terminal's Ctrl+Enter
returns to its standard InsertNewLineIfMultiLine action.
Adds a regression unit test that opens then closes Rich Input and
asserts the editor's enter_settings.ctrl_enter is back to
InsertNewLineIfMultiLine. Integration coverage was omitted because
closing Rich Input from the integration harness isn't cleanly
exposed; the unit test calls CLIAgentSessionsModel::close_input
directly — the same path production code uses.
Also exposes a minimal pub fn enter_settings(&self) -> EnterSettings
getter on EditorView so the test can inspect the field without
simulating a full keystroke dispatch.
refs warpdotdev#11588
|
@oz-for-oss good catch — third in a row. This was a side effect of the helper introduced in the previous fix. Fixed in 993327d. Root cause (matching your trace): the editor view is shared between Rich Input (CLI agent) mode and normal terminal mode. The Fix: branch the helper on
Regression coverage (unit): I added a small Integration coverage was omitted for this specific case because closing Rich Input cleanly from the integration harness isn't an existing exposed transition; the unit test is the load-bearing regression here. 11/11 unit tests pass; full verification suite green. 🤖 Generated by Claude Code on behalf of @cbeaulieu-gt |
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR adds a synced AI setting and settings-page toggle for submitting CLI-agent Rich Input with Ctrl+Enter, updates the input/editor event handling so Enter inserts newlines when enabled while preserving menu acceptance and shell-mode submit behavior, and adds unit plus integration coverage for the key behavior matrix and regressions.
Concerns
No blocking concerns found. The PR description includes user-facing visual evidence, and I did not identify security issues or spec drift from the provided context.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
@advait-m Hey, just seeing if there was anything else needed from me before this could move to the next stage? |
ah sorry, wasn't getting pings since I wasn't assigned as a reviewer. Added this to my todo list, should get to it later today/tomorrow to take a look! |
advait-m
left a comment
There was a problem hiding this comment.
looking good overall! 2 core pieces:
- Let's make sure not to regress existing ctrl-enter newline behavior
- Let's cut down comments added a ton here - getting very verbose
|
(I tested the core behavior with the setting, looking good there!) |
…r review (warpdotdev#11723) Part A: `update_cli_agent_enter_settings` now sets `ctrl_enter: InsertNewLineIfMultiLine` when `submit_on_ctrl_enter=false` (previously always `Emit`, making the editor's ctrl_enter a no-op). Also re-invokes `update_cli_agent_enter_settings` when the `SubmitRichInputOnCtrlEnter` setting changes so a mid-session toggle takes effect immediately. Part B (TDD): added regression test `ctrl_enter_inserts_newline_when_submit_on_ctrl_enter_is_false` — red against pre-fix code, green after. Parts C+D: trimmed unit tests (11→9 kept + 1 new) and integration tests (6→2) per reviewer advait-m's agreed keep/cut/merge list; shortened verbose docstrings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aggressive comment-only cleanup of the Rich Input submit_on_ctrl_enter test region in input_tests.rs: drop divider banners and self-documenting test docstrings, remove inline comments that restate assertion messages. Keep the regression rationale on the toggle-OFF Ctrl+Enter test. No test logic or behavior changed (2 insertions, 93 deletions). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment-only cleanup of the remaining PR-added comment blocks: aggressive trim of test-helper docstrings in integration_testing/input/step.rs (incl. fixing an inaccurate is-empty doc), condense public editor API docs in editor/view/mod.rs to one line, and de-duplicate the production input.rs comments while keeping the selection-asymmetry rationale. No logic changed (13 insertions, 47 deletions). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@advait-m I updated the change to have the following behaviour: Toggle Off Toggle On Also trimmed the comments and tests to a more concise set. Let me know if you more trimming or any other changes. |
…-rich-input-ctrl-enter # Conflicts: # app/src/integration_testing/input/step.rs
…h_notification field Upstream commit 63fe728 added `received_rich_notification: bool` to `CLIAgentSession`. Two feature-branch files that construct the struct directly were not updated during the merge, causing a compile error. Add `received_rich_notification: false` to each initializer.
Plain `cargo fmt` misses the repo's unstable import rules
(imports_granularity=Module, group_imports=StdExternalCrate); running
script/format flattens the nested warp::{...} block, fixing the CI
Formatting + Clippy failure.
Co-Authored-By: Oz <oz-agent@warp.dev>
|
Saw a formatting failure from CI - pushed up a commit to fix Looking otherwise |
The integration suite was trimmed to two tests, but ui_tests.rs still referenced five removed names (and omitted the new wiring-guard test). Those stale entries would panic at runtime with "test not found". List exactly the two registered tests instead. Co-Authored-By: Oz <oz-agent@warp.dev>
|
and a slight test trimming fix, it should pass CI now |
After trimming the rich-input integration suite, this assertion helper has no callers. Drop the dead code. Co-Authored-By: Oz <oz-agent@warp.dev>
advait-m
left a comment
There was a problem hiding this comment.
awesome, thanks for iterating with me here + the contribution @cbeaulieu-gt !
Description
Adds a user-facing toggle in Settings → Agents → Third Party Agents that switches the Rich Input editor's submit binding from
Enter(default) toCtrl+Enter. When the toggle is on,Enterinserts a newline andCtrl+Entersubmits — the IDE-style convention that makes multi-line prompt composition for CLI agents much less awkward.Default is
false(current behavior preserved). The setting syncs globally and persists across restarts through the standard AI-settings path.Linked Issue
Closes #11588
ready-to-specorready-to-implement. (ready-to-implement)Testing
Automated tests
Unit tests —
app/src/terminal/input_tests.rs(new file, ~531 lines). Covers the 2×2 keystroke × setting-state matrix plus shell-mode and keymap-context regressions:submit_on_ctrl_enter_setting_defaults_to_false_and_round_tripsenter_submits_when_submit_on_ctrl_enter_is_falsectrl_enter_emits_ctrl_enter_event_when_submit_on_ctrl_enter_is_falseenter_inserts_newline_when_submit_on_ctrl_enter_is_truectrl_enter_submits_when_submit_on_ctrl_enter_is_true(asserts buffer is cleared after submit)ctrl_enter_with_shell_mode_locked_prepends_prefix_and_exits_shell_mode_when_setting_trueIntegration tests —
crates/integration/src/test/rich_input_ctrl_enter.rs(new file, 4 tests). These drive real keystrokes through the editor event-dispatch pipeline rather than callinginput_ctrl_enterdirectly, which is what the editor'sInsertNewLineIfMultiLine-then-backspace sequence actually needs to be exercised against:test_rich_input_enter_submits_when_ctrl_enter_setting_is_falsetest_rich_input_ctrl_enter_passthrough_when_ctrl_enter_setting_is_falsetest_rich_input_enter_inserts_newline_when_ctrl_enter_setting_is_truetest_rich_input_ctrl_enter_submits_when_ctrl_enter_setting_is_trueManual testing
./script/runScreenshots / Videos
warp-rich-toggle-editor.mp4
Agent Mode
CHANGELOG-IMPROVEMENT: Added a setting to submit Rich Input on Ctrl+Enter, so Enter inserts a newline for easier multi-line prompt composition with CLI agents.
🤖 Generated by Claude Code on behalf of @cbeaulieu-gt