mirror of
https://github.com/openai/codex.git
synced 2026-04-24 22:54:54 +00:00
Prevent backspace from removing a text element when the cursor is at the element’s left edge (#9630)
**Summary** - Prevent backspace from removing a text element when the cursor is at the element’s left edge. - Instead just delete the char before the placeholder (moving it to the left).
This commit is contained in:
committed by
GitHub
parent
e2bd9311c9
commit
279c9534a1
@@ -2039,15 +2039,6 @@ impl ChatComposer {
|
|||||||
{
|
{
|
||||||
self.handle_paste(pasted);
|
self.handle_paste(pasted);
|
||||||
}
|
}
|
||||||
// Backspace at the start of an image placeholder should delete that placeholder (rather
|
|
||||||
// than deleting content before it). Do this without scanning the full text by consulting
|
|
||||||
// the textarea's element list.
|
|
||||||
if matches!(input.code, KeyCode::Backspace)
|
|
||||||
&& self.try_remove_image_element_at_cursor_start()
|
|
||||||
{
|
|
||||||
return (InputResult::None, true);
|
|
||||||
}
|
|
||||||
|
|
||||||
// For non-char inputs (or after flushing), handle normally.
|
// For non-char inputs (or after flushing), handle normally.
|
||||||
// Track element removals so we can drop any corresponding placeholders without scanning
|
// Track element removals so we can drop any corresponding placeholders without scanning
|
||||||
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
|
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
|
||||||
@@ -2086,29 +2077,6 @@ impl ChatComposer {
|
|||||||
(InputResult::None, true)
|
(InputResult::None, true)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn try_remove_image_element_at_cursor_start(&mut self) -> bool {
|
|
||||||
if self.attached_images.is_empty() {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
let p = self.textarea.cursor();
|
|
||||||
let Some(payload) = self.textarea.element_payload_starting_at(p) else {
|
|
||||||
return false;
|
|
||||||
};
|
|
||||||
let Some(idx) = self
|
|
||||||
.attached_images
|
|
||||||
.iter()
|
|
||||||
.position(|img| img.placeholder == payload)
|
|
||||||
else {
|
|
||||||
return false;
|
|
||||||
};
|
|
||||||
|
|
||||||
self.textarea.replace_range(p..p + payload.len(), "");
|
|
||||||
self.attached_images.remove(idx);
|
|
||||||
self.relabel_attached_images_and_update_placeholders();
|
|
||||||
true
|
|
||||||
}
|
|
||||||
|
|
||||||
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
|
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
|
||||||
let elements_after: HashSet<String> =
|
let elements_after: HashSet<String> =
|
||||||
self.textarea.element_payloads().into_iter().collect();
|
self.textarea.element_payloads().into_iter().collect();
|
||||||
@@ -4662,8 +4630,7 @@ mod tests {
|
|||||||
assert!(!composer.textarea.text().contains(&placeholder));
|
assert!(!composer.textarea.text().contains(&placeholder));
|
||||||
assert!(composer.attached_images.is_empty());
|
assert!(composer.attached_images.is_empty());
|
||||||
|
|
||||||
// Re-add and test backspace in middle: should break the placeholder string
|
// Re-add and ensure backspace at element start does not delete the placeholder.
|
||||||
// and drop the image mapping (same as text placeholder behavior).
|
|
||||||
composer.attach_image(path);
|
composer.attach_image(path);
|
||||||
let placeholder2 = composer.attached_images[0].placeholder.clone();
|
let placeholder2 = composer.attached_images[0].placeholder.clone();
|
||||||
// Move cursor to roughly middle of placeholder
|
// Move cursor to roughly middle of placeholder
|
||||||
@@ -4671,8 +4638,8 @@ mod tests {
|
|||||||
let mid_pos = start_pos + (placeholder2.len() / 2);
|
let mid_pos = start_pos + (placeholder2.len() / 2);
|
||||||
composer.textarea.set_cursor(mid_pos);
|
composer.textarea.set_cursor(mid_pos);
|
||||||
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
|
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
|
||||||
assert!(!composer.textarea.text().contains(&placeholder2));
|
assert!(composer.textarea.text().contains(&placeholder2));
|
||||||
assert!(composer.attached_images.is_empty());
|
assert_eq!(composer.attached_images.len(), 1);
|
||||||
} else {
|
} else {
|
||||||
panic!("Placeholder not found in textarea");
|
panic!("Placeholder not found in textarea");
|
||||||
}
|
}
|
||||||
@@ -4852,7 +4819,7 @@ mod tests {
|
|||||||
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
|
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
|
||||||
assert_eq!(composer.attached_images.len(), 2);
|
assert_eq!(composer.attached_images.len(), 2);
|
||||||
|
|
||||||
// Delete the first element using normal textarea editing (Delete at cursor start).
|
// Delete the first element using normal textarea editing (forward Delete at cursor start).
|
||||||
composer.textarea.set_cursor(0);
|
composer.textarea.set_cursor(0);
|
||||||
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
|
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
|
||||||
|
|
||||||
|
|||||||
@@ -766,12 +766,6 @@ impl TextArea {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn element_payload_starting_at(&self, pos: usize) -> Option<String> {
|
|
||||||
let pos = pos.min(self.text.len());
|
|
||||||
let elem = self.elements.iter().find(|e| e.range.start == pos)?;
|
|
||||||
self.text.get(elem.range.clone()).map(str::to_string)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Renames a single text element in-place, keeping it atomic.
|
/// Renames a single text element in-place, keeping it atomic.
|
||||||
///
|
///
|
||||||
/// Use this when the element payload is an identifier (e.g. a placeholder) that must be
|
/// Use this when the element payload is an identifier (e.g. a placeholder) that must be
|
||||||
@@ -1327,6 +1321,21 @@ mod tests {
|
|||||||
assert_eq!(t.text(), "b");
|
assert_eq!(t.text(), "b");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn delete_forward_deletes_element_at_left_edge() {
|
||||||
|
let mut t = TextArea::new();
|
||||||
|
t.insert_str("a");
|
||||||
|
t.insert_element("<element>");
|
||||||
|
t.insert_str("b");
|
||||||
|
|
||||||
|
let elem_start = t.elements[0].range.start;
|
||||||
|
t.set_cursor(elem_start);
|
||||||
|
t.delete_forward(1);
|
||||||
|
|
||||||
|
assert_eq!(t.text(), "ab");
|
||||||
|
assert_eq!(t.cursor(), elem_start);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn delete_backward_word_and_kill_line_variants() {
|
fn delete_backward_word_and_kill_line_variants() {
|
||||||
// delete backward word at end removes the whole previous word
|
// delete backward word at end removes the whole previous word
|
||||||
|
|||||||
Reference in New Issue
Block a user