Skip to content

Cmd+Enter keybinding for Send to Agent in code review#10295

Open
alokedesai wants to merge 1 commit intoaloke/cursor_and_placeholderfrom
aloke/cmd_enter_send_to_agent
Open

Cmd+Enter keybinding for Send to Agent in code review#10295
alokedesai wants to merge 1 commit intoaloke/cursor_and_placeholderfrom
aloke/cmd_enter_send_to_agent

Conversation

@alokedesai
Copy link
Copy Markdown
Member

Description

Linked Issue

  • The linked issue is labeled ready-to-spec or ready-to-implement.
  • Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes).

Screenshots / Videos

Testing

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

- Add EditorIsEditable guard on CmdEnter binding to prevent read-only
  editors from consuming the keystroke
- CodeEditorView reclaims focus on CommentEditor close to break stale
  focus cycle
- CodeReviewView focus_self after SubmitComments
- Register SubmitComments as EditableBinding with dynamic keybinding
- Convert send button to ActionButton with KeystrokeSource::Binding
- Restore send button enablement state with AI/destination/comment checks
- Subscribe to AIRequestUsageModel for credit change updates

Co-Authored-By: Oz <oz-agent@warp.dev>
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 6, 2026

@alokedesai

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Member Author

alokedesai commented May 6, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR adds a Cmd/Ctrl+Enter binding and an ActionButton-backed Send to Agent button for code review comments.

Concerns

  • The new shortcut path bypasses the button's disabled-state checks, so keyboard submission can still run when sending is unavailable.
  • PR-info refresh guards were removed from Create/View PR paths, which can expose stale or invalid PR actions during refresh.
  • FreeBSD was accidentally removed from the unsaved-changes modal platform guard.
  • This is a user-visible UI/interaction change, but the PR description does not include screenshots or a video. For faster review, please upload screenshots or a video of the feature working end to end.

Verdict

Found: 0 critical, 4 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

&& !diff_state.is_on_main_branch()
&& upstream_differs_from_main
{
} else if has_upstream && !diff_state.is_on_main_branch() && upstream_differs_from_main {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ [IMPORTANT] Dropping is_pr_info_refreshing lets the primary button show Create PR while PR discovery is still in flight, so a branch with an existing PR can open the create dialog before refresh completes. Keep the refresh guard here and in the matching Create PR paths.

ctx.notify();
}
CodeReviewAction::SubmitComments => {
self.handle_submit_review_with_comments(ctx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ [IMPORTANT] The shortcut path calls submit directly, bypassing the disabled state computed for send_button; Cmd+Enter can still try to send when there is no destination or no Warp AI availability. Gate this action with the same sendability predicate as the button.

target_os = "freebsd",
target_os = "windows"
)
any(target_os = "linux", target_os = "windows")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ [IMPORTANT] This drops FreeBSD from the unsaved-changes modal branch; on FreeBSD the close handler enters the unsaved-changes block but shows no dialog and does not close. Restore the platform in this cfg.

Suggested change
any(target_os = "linux", target_os = "windows")
any(
target_os = "linux",
target_os = "freebsd",
target_os = "windows"
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant