Fix crash in manipulate_text on multibuffers (#57165)
In `Editor::manipulate_text`, we computed selection boundaries for the updated text assuming the requested edit would be applied exactly. This is not always true. As a result, we could produce an invalid selection range and panic. This change replaces manual selection boundary computation with anchors. It also skips edits when `new_text == old_text`. Closes FR-10. Release Notes: - N/A
This commit is contained in:
1 parent
deb6f199b0
commit
88fc689cfe
2 files changed
+65
-10
No files matched your search
+10
-10
@@ -6771,7 +6771,6 @@ impl Editor {
|
||||
|
||||
let mut new_selections = Vec::new();
|
||||
let mut edits = Vec::new();
|
||||
let mut selection_adjustment = 0isize;
|
||||
|
||||
for selection in self.selections.all_adjusted(&self.display_snapshot(cx)) {
|
||||
let selection_is_empty = selection.is_empty();
|
||||
@@ -6786,23 +6785,24 @@ impl Editor {
|
||||
)
|
||||
};
|
||||
|
||||
let text = buffer.text_for_range(start..end).collect::<String>();
|
||||
let old_length = text.len() as isize;
|
||||
let text = callback(&text);
|
||||
let old_text = buffer.text_for_range(start..end).collect::<String>();
|
||||
let new_text = callback(&old_text);
|
||||
|
||||
new_selections.push(Selection {
|
||||
start: MultiBufferOffset((start.0 as isize - selection_adjustment) as usize),
|
||||
end: MultiBufferOffset(
|
||||
((start.0 + text.len()) as isize - selection_adjustment) as usize,
|
||||
),
|
||||
start: buffer.anchor_before(start),
|
||||
end: buffer.anchor_after(end),
|
||||
goal: SelectionGoal::None,
|
||||
id: selection.id,
|
||||
reversed: selection.reversed,
|
||||
});
|
||||
|
||||
selection_adjustment += old_length - text.len() as isize;
|
||||
if new_text != old_text {
|
||||
edits.push((start..end, new_text));
|
||||
}
|
||||
}
|
||||
|
||||
edits.push((start..end, text));
|
||||
if edits.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
self.transact(window, cx, |this, window, cx| {
|
||||
|
||||
@@ -6898,6 +6898,61 @@ async fn test_convert_to_base64(cx: &mut TestAppContext) {
|
||||
"});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_manipulate_text_handles_cross_excerpt_edit_that_applies_differently(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
init_test(cx, |_| {});
|
||||
|
||||
let buffer_1 = cx.new(|cx| {
|
||||
let mut buffer = Buffer::local("ab", cx);
|
||||
// The selected multibuffer range starts in this excerpt, but edits to
|
||||
// it are skipped because the underlying buffer is read-only.
|
||||
buffer.set_capability(language::Capability::ReadOnly, cx);
|
||||
buffer
|
||||
});
|
||||
let buffer_2 = cx.new(|cx| Buffer::local("cd", cx));
|
||||
let multibuffer = cx.new(|cx| {
|
||||
let mut multibuffer = MultiBuffer::new(ReadWrite);
|
||||
multibuffer.set_excerpts_for_path(
|
||||
PathKey::sorted(0),
|
||||
buffer_1.clone(),
|
||||
[Point::new(0, 0)..Point::new(0, 2)],
|
||||
0,
|
||||
cx,
|
||||
);
|
||||
multibuffer.set_excerpts_for_path(
|
||||
PathKey::sorted(1),
|
||||
buffer_2.clone(),
|
||||
[Point::new(0, 0)..Point::new(0, 2)],
|
||||
0,
|
||||
cx,
|
||||
);
|
||||
multibuffer
|
||||
});
|
||||
|
||||
cx.add_window(|window, cx| {
|
||||
let mut editor = build_editor(multibuffer, window, cx);
|
||||
let len = editor.buffer().read(cx).len(cx);
|
||||
editor.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| {
|
||||
selections.select_ranges([MultiBufferOffset(0)..len])
|
||||
});
|
||||
|
||||
// No-op transformations should not be sent through `MultiBuffer::edit`.
|
||||
editor.manipulate_text(window, cx, |text| text.to_string());
|
||||
assert_eq!(buffer_1.read(cx).text(), "ab");
|
||||
assert_eq!(buffer_2.read(cx).text(), "cd");
|
||||
|
||||
// A real replacement can apply differently than requested; selection
|
||||
// remapping should follow the actual edit instead of predicted offsets.
|
||||
editor.manipulate_text(window, cx, |_| "replacement".to_string());
|
||||
assert_eq!(buffer_1.read(cx).text(), "ab");
|
||||
assert_eq!(buffer_2.read(cx).text(), "");
|
||||
|
||||
editor
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_manipulate_text(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
||||
Reference in new issue
Block a user