mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Prevent multiline report notes from submitting early (#45116)
## Why Unbracketed multiline pastes could submit the report form at the first newline, before the full note was entered. ## What changed Use `PasteBurst` in the report note editor to insert newlines during rapid paste input and allow `Enter` to submit after the burst ends. Extend the burst window across newlines and tabs, and clear burst state after explicit pastes or other editing input. ## Testing Add regression tests for multiline paste bursts with short first lines, blank lines, and tabs, verifying the complete submitted note with and without logs. Add a multiline rendering snapshot and a test that bracketed paste stays in the report form, leaves the chat composer empty, and allows immediate submission. GitOrigin-RevId: a76e680a01767c619a4fcc150d1dcad537e67194
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
//! Collects a feedback note with audience-specific disclosure and independent log consent.
|
||||
//! The editor and submission controls remain visible when the disclosure must scroll.
|
||||
//! Unbracketed paste bursts insert newlines; Enter submits after the burst ends.
|
||||
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
@@ -15,11 +16,13 @@ use ratatui::widgets::StatefulWidgetRef;
|
||||
use ratatui::widgets::Widget;
|
||||
use std::cell::Cell;
|
||||
use std::cell::RefCell;
|
||||
use std::time::Instant;
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event::FeedbackCategory;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
use crate::key_hint;
|
||||
use crate::key_hint::has_ctrl_or_alt;
|
||||
use crate::render::renderable::Renderable;
|
||||
use crate::terminal_hyperlinks::mark_underlined_hyperlink;
|
||||
use crate::wrapping::RtOptions;
|
||||
@@ -28,6 +31,7 @@ use crate::wrapping::word_wrap_lines;
|
||||
use super::CancellationEvent;
|
||||
use super::bottom_pane_view::BottomPaneView;
|
||||
use super::feedback_view::FeedbackAudience;
|
||||
use super::paste_burst::PasteBurst;
|
||||
use super::popup_consts::accept_cancel_hint_line;
|
||||
use super::textarea::TextArea;
|
||||
use super::textarea::TextAreaState;
|
||||
@@ -52,6 +56,7 @@ pub(crate) struct FeedbackNoteView {
|
||||
// UI state
|
||||
textarea: TextArea,
|
||||
textarea_state: RefCell<TextAreaState>,
|
||||
paste_burst: PasteBurst,
|
||||
intro_scroll: Cell<usize>,
|
||||
intro_page_height: Cell<usize>,
|
||||
complete: bool,
|
||||
@@ -73,6 +78,7 @@ impl FeedbackNoteView {
|
||||
feedback_audience,
|
||||
textarea: TextArea::new(),
|
||||
textarea_state: RefCell::new(TextAreaState::default()),
|
||||
paste_burst: PasteBurst::default(),
|
||||
intro_scroll: Cell::new(0),
|
||||
intro_page_height: Cell::new(1),
|
||||
complete: false,
|
||||
@@ -90,10 +96,8 @@ impl FeedbackNoteView {
|
||||
});
|
||||
self.complete = true;
|
||||
}
|
||||
}
|
||||
|
||||
impl BottomPaneView for FeedbackNoteView {
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
fn handle_key_event_at(&mut self, key_event: KeyEvent, now: Instant) {
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::PageUp,
|
||||
@@ -122,22 +126,52 @@ impl BottomPaneView for FeedbackNoteView {
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Enter,
|
||||
modifiers: KeyModifiers::NONE,
|
||||
modifiers,
|
||||
..
|
||||
} => {
|
||||
self.submit();
|
||||
if self.paste_burst.direct_insert_newline_should_insert(now) {
|
||||
self.paste_burst.extend_window(now);
|
||||
self.textarea.insert_str("\n");
|
||||
} else if modifiers == KeyModifiers::NONE {
|
||||
self.submit();
|
||||
} else {
|
||||
self.textarea.input(key_event);
|
||||
}
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Enter,
|
||||
code: KeyCode::Char(_),
|
||||
modifiers,
|
||||
..
|
||||
} => {
|
||||
} if !has_ctrl_or_alt(modifiers) => {
|
||||
let paste_like_burst = self.paste_burst.on_plain_char_no_hold(now).is_some();
|
||||
self.textarea.input(key_event);
|
||||
if paste_like_burst {
|
||||
self.paste_burst.extend_window(now);
|
||||
}
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Tab,
|
||||
modifiers,
|
||||
..
|
||||
} if !has_ctrl_or_alt(modifiers) => {
|
||||
let in_paste_burst = self.paste_burst.direct_insert_newline_should_insert(now);
|
||||
self.textarea.input(key_event);
|
||||
if in_paste_burst {
|
||||
self.paste_burst.extend_window(now);
|
||||
}
|
||||
}
|
||||
other => {
|
||||
self.textarea.input(other);
|
||||
self.paste_burst.clear_after_explicit_paste();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl BottomPaneView for FeedbackNoteView {
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
self.handle_key_event_at(key_event, Instant::now());
|
||||
}
|
||||
|
||||
fn on_ctrl_c(&mut self) -> CancellationEvent {
|
||||
self.complete = true;
|
||||
@@ -153,6 +187,7 @@ impl BottomPaneView for FeedbackNoteView {
|
||||
return false;
|
||||
}
|
||||
self.textarea.insert_str(&pasted);
|
||||
self.paste_burst.clear_after_explicit_paste();
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::time::Duration;
|
||||
|
||||
fn render(view: &FeedbackNoteView, width: u16) -> String {
|
||||
let height = view.desired_height(width);
|
||||
@@ -221,6 +222,105 @@ fn feedback_disclosure_keeps_editor_visible_in_four_rows() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feedback_paste_burst_preserves_multiline_note_until_submit() {
|
||||
for include_logs in [false, true] {
|
||||
for note in ["x\nrest", "id\n\nbody", "foo\nbar", "x\t\nrest"] {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
let mut view = FeedbackNoteView::new(
|
||||
FeedbackCategory::Bug,
|
||||
/*turn_id*/ None,
|
||||
AppEventSender::new(tx),
|
||||
include_logs,
|
||||
FeedbackAudience::External,
|
||||
);
|
||||
let now = Instant::now();
|
||||
for (index, ch) in note.chars().enumerate() {
|
||||
let code = match ch {
|
||||
'\n' => KeyCode::Enter,
|
||||
'\t' => KeyCode::Tab,
|
||||
ch => KeyCode::Char(ch),
|
||||
};
|
||||
view.handle_key_event_at(
|
||||
KeyEvent::from(code),
|
||||
now + Duration::from_millis(index as u64),
|
||||
);
|
||||
assert!(!view.is_complete());
|
||||
assert!(rx.try_recv().is_err());
|
||||
}
|
||||
let expected_note = note.replace('\t', "");
|
||||
assert_eq!(view.textarea.text(), expected_note);
|
||||
if !include_logs && note == "foo\nbar" {
|
||||
insta::assert_snapshot!("feedback_multiline_paste", render(&view, /*width*/ 60));
|
||||
}
|
||||
|
||||
view.handle_key_event_at(
|
||||
KeyEvent::from(KeyCode::Enter),
|
||||
now + Duration::from_millis(200),
|
||||
);
|
||||
|
||||
let AppEvent::SubmitFeedback {
|
||||
category,
|
||||
reason,
|
||||
turn_id,
|
||||
include_logs: submitted_logs,
|
||||
} = rx.try_recv().expect("submit feedback event")
|
||||
else {
|
||||
panic!("expected feedback submission");
|
||||
};
|
||||
assert_eq!(
|
||||
(category, reason, turn_id, submitted_logs),
|
||||
(
|
||||
FeedbackCategory::Bug,
|
||||
Some(expected_note),
|
||||
None,
|
||||
include_logs
|
||||
)
|
||||
);
|
||||
assert!(view.is_complete());
|
||||
assert!(rx.try_recv().is_err());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn feedback_bracketed_paste_does_not_reach_chat_composer() {
|
||||
for include_logs in [false, true] {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut pane = super::super::tests::test_pane(tx.clone());
|
||||
pane.show_view(Box::new(FeedbackNoteView::new(
|
||||
FeedbackCategory::Bug,
|
||||
/*turn_id*/ None,
|
||||
tx,
|
||||
include_logs,
|
||||
FeedbackAudience::External,
|
||||
)));
|
||||
// An explicit paste ends any preceding keystroke burst.
|
||||
pane.handle_key_event(KeyEvent::from(KeyCode::Char('x')));
|
||||
pane.handle_paste("\nrest".to_string());
|
||||
assert!(rx.try_recv().is_err());
|
||||
assert!(matches!(
|
||||
pane.handle_key_event(KeyEvent::from(KeyCode::Enter)),
|
||||
super::super::InputResult::None
|
||||
));
|
||||
let AppEvent::SubmitFeedback {
|
||||
reason,
|
||||
include_logs: submitted_logs,
|
||||
..
|
||||
} = rx.try_recv().expect("submit feedback event")
|
||||
else {
|
||||
panic!("expected feedback submission");
|
||||
};
|
||||
assert_eq!(
|
||||
(reason, submitted_logs),
|
||||
(Some("x\nrest".into()), include_logs)
|
||||
);
|
||||
assert!(pane.composer_is_empty());
|
||||
assert!(rx.try_recv().is_err());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn submit_feedback_emits_submit_event_with_trimmed_note() {
|
||||
let (tx_raw, mut rx) = tokio::sync::mpsc::unbounded_channel::<AppEvent>();
|
||||
|
||||
+12
@@ -0,0 +1,12 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/feedback_note_view_tests.rs
|
||||
expression: "render(&view, 60)"
|
||||
---
|
||||
▌ Tell us more (bug)
|
||||
▌
|
||||
▌ Your feedback can be used to improve ChatGPT. Learn more.
|
||||
▌
|
||||
▌ foo
|
||||
▌ bar
|
||||
|
||||
Press enter to submit or esc to cancel
|
||||
Reference in New Issue
Block a user