Skip to content

Prevent backspace from removing a text element when the cursor is at the element’s left edge #9630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
charley-oai merged 2 commits into from
Jan 24, 2026
Merged

Prevent backspace from removing a text element when the cursor is at the element’s left edge #9630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6391b64
Prevent delete at left edge of placeholder from removing the placehol…
charley-oai Jan 21, 2026
70a9600
Distinguish between forward/backward delete
charley-oai Jan 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 4 additions & 37 deletions codex-rs/tui/src/bottom_pane/chat_composer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2039,15 +2039,6 @@ impl ChatComposer {
{
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.
// Track element removals so we can drop any corresponding placeholders without scanning
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
Expand Down Expand Up @@ -2086,29 +2077,6 @@ impl ChatComposer {
(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>) {
let elements_after: HashSet<String> =
self.textarea.element_payloads().into_iter().collect();
Expand Down Expand Up @@ -4662,17 +4630,16 @@ mod tests {
assert!(!composer.textarea.text().contains(&placeholder));
assert!(composer.attached_images.is_empty());

// Re-add and test backspace in middle: should break the placeholder string
// and drop the image mapping (same as text placeholder behavior).
// Re-add and ensure backspace at element start does not delete the placeholder.
composer.attach_image(path);
let placeholder2 = composer.attached_images[0].placeholder.clone();
// Move cursor to roughly middle of placeholder
if let Some(start_pos) = composer.textarea.text().find(&placeholder2) {
let mid_pos = start_pos + (placeholder2.len() / 2);
composer.textarea.set_cursor(mid_pos);
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert!(!composer.textarea.text().contains(&placeholder2));
assert!(composer.attached_images.is_empty());
assert!(composer.textarea.text().contains(&placeholder2));
assert_eq!(composer.attached_images.len(), 1);
} else {
panic!("Placeholder not found in textarea");
}
Expand Down Expand Up @@ -4852,7 +4819,7 @@ mod tests {
assert_eq!(composer.textarea.text(), "[Image #1][Image #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.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));

Expand Down
21 changes: 15 additions & 6 deletions codex-rs/tui/src/bottom_pane/textarea.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,12 +766,6 @@ impl TextArea {
.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.
///
/// Use this when the element payload is an identifier (e.g. a placeholder) that must be
Expand Down Expand Up @@ -1327,6 +1321,21 @@ mod tests {
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]
fn delete_backward_word_and_kill_line_variants() {
// delete backward word at end removes the whole previous word
Expand Down
Loading