Fix the display point out of range panic due to wrong coordinate space used (#62055)
Closes ZED-AKV Closes ZED-ANB Follow-up to https://github.com/zed-industries/zed/pull/61153 The original PR mixed up the coordinate spaces, and the newly added methods used buffer rows (as the existing code did before that PR), but the new code uses `FoldMap` which expects a tab row instead of the buffer one. Release Notes: - Fixed a panic when selections are added by tab-expanded column
This commit is contained in:
1 parent
01961bef59
commit
db73efafe4
4 files changed
+145
-24
No files matched your search
@@ -1547,7 +1547,7 @@ impl DisplaySnapshot {
|
||||
/// The column `point` sits at once tabs are expanded, which is where it appears
|
||||
/// on screen when the line isn't soft-wrapped. Unlike a display column this is
|
||||
/// counted from the start of the buffer row rather than the wrapped segment.
|
||||
pub fn tab_expanded_column(&self, point: Point) -> u32 {
|
||||
pub(crate) fn tab_expanded_column(&self, point: Point) -> u32 {
|
||||
self.tab_snapshot()
|
||||
.point_to_tab_point(point, Bias::Left)
|
||||
.0
|
||||
@@ -1555,15 +1555,21 @@ impl DisplaySnapshot {
|
||||
}
|
||||
|
||||
/// Inverse of [`Self::tab_expanded_column`], clamped to the end of the row.
|
||||
pub fn point_for_tab_expanded_column(&self, row: u32, column: u32) -> Point {
|
||||
let column = column.min(self.tab_snapshot().line_len(row));
|
||||
self.tab_snapshot()
|
||||
.tab_point_to_point(TabPoint(Point::new(row, column)), Bias::Left)
|
||||
pub(crate) fn point_for_tab_expanded_column(
|
||||
&self,
|
||||
buffer_row: MultiBufferRow,
|
||||
column: u32,
|
||||
) -> Point {
|
||||
let tab_snapshot = self.tab_snapshot();
|
||||
let tab_row = tab_snapshot.buffer_row_to_tab_row(buffer_row);
|
||||
let column = column.min(tab_snapshot.line_len(tab_row));
|
||||
tab_snapshot.tab_point_to_point(TabPoint(Point::new(tab_row, column)), Bias::Left)
|
||||
}
|
||||
|
||||
/// The length of `row` once tabs are expanded.
|
||||
pub fn tab_expanded_line_len(&self, row: u32) -> u32 {
|
||||
self.tab_snapshot().line_len(row)
|
||||
/// The length of `buffer_row` once tabs are expanded.
|
||||
pub(crate) fn tab_expanded_line_len(&self, buffer_row: MultiBufferRow) -> u32 {
|
||||
let tab_snapshot = self.tab_snapshot();
|
||||
tab_snapshot.line_len(tab_snapshot.buffer_row_to_tab_row(buffer_row))
|
||||
}
|
||||
|
||||
pub fn fold_snapshot(&self) -> &FoldSnapshot {
|
||||
|
||||
@@ -4,7 +4,7 @@ use super::{
|
||||
};
|
||||
|
||||
use language::{LanguageAwareStyling, Point};
|
||||
use multi_buffer::MultiBufferSnapshot;
|
||||
use multi_buffer::{MultiBufferRow, MultiBufferSnapshot};
|
||||
use std::{cmp, num::NonZeroU32, ops::Range};
|
||||
use sum_tree::Bias;
|
||||
|
||||
@@ -392,6 +392,11 @@ impl TabSnapshot {
|
||||
)
|
||||
}
|
||||
|
||||
pub fn buffer_row_to_tab_row(&self, buffer_row: MultiBufferRow) -> u32 {
|
||||
self.point_to_tab_point(Point::new(buffer_row.0, 0), Bias::Left)
|
||||
.row()
|
||||
}
|
||||
|
||||
#[ztracing::instrument(skip_all)]
|
||||
pub fn point_to_tab_point(&self, point: Point, bias: Bias) -> TabPoint {
|
||||
let inlay_point = self.fold_snapshot.inlay_snapshot.to_inlay_point(point);
|
||||
|
||||
@@ -10774,7 +10774,6 @@ async fn test_split_selection_into_lines_interacting_with_creases(cx: &mut TestA
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
/// A different number of tabs can align the same column on each row, so a cursor has to
|
||||
/// be placed by the column the tabs expand to. Counting a tab as a single column lands it
|
||||
/// wherever that many characters happen to reach on the next row.
|
||||
@@ -10808,6 +10807,115 @@ async fn test_add_selection_below_with_tab_aligned_columns(cx: &mut TestAppConte
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
/// Regression test for a panic ("display point out of range"): with a multi-line fold,
|
||||
/// buffer rows below the fold exceed the fold map's max row, so they must be converted
|
||||
/// to tab map rows instead of being used directly.
|
||||
async fn test_add_selection_above_below_with_fold(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
||||
let mut cx = EditorTestContext::new(cx).await;
|
||||
|
||||
cx.set_state(indoc!(
|
||||
r#"fn foo() {
|
||||
aaaa
|
||||
bbbb
|
||||
}
|
||||
one
|
||||
twoˇ"#
|
||||
));
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
editor.fold_creases(
|
||||
vec![Crease::simple(
|
||||
Point::new(0, 10)..Point::new(3, 0),
|
||||
FoldPlaceholder::test(),
|
||||
)],
|
||||
true,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
editor.add_selection_above(
|
||||
&AddSelectionAbove {
|
||||
skip_soft_wrap: true,
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.assert_editor_state(indoc!(
|
||||
r#"fn foo() {
|
||||
aaaa
|
||||
bbbb
|
||||
}
|
||||
oneˇ
|
||||
twoˇ"#
|
||||
));
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
editor.add_selection_above(
|
||||
&AddSelectionAbove {
|
||||
skip_soft_wrap: true,
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.assert_editor_state(indoc!(
|
||||
r#"fn ˇfoo() {
|
||||
aaaa
|
||||
bbbb
|
||||
}
|
||||
oneˇ
|
||||
twoˇ"#
|
||||
));
|
||||
|
||||
cx.set_state(indoc!(
|
||||
r#"fn fˇoo() {
|
||||
aaaa
|
||||
bbbb
|
||||
}
|
||||
one
|
||||
two"#
|
||||
));
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
editor.fold_creases(
|
||||
vec![Crease::simple(
|
||||
Point::new(0, 10)..Point::new(3, 0),
|
||||
FoldPlaceholder::test(),
|
||||
)],
|
||||
true,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
editor.add_selection_below(
|
||||
&AddSelectionBelow {
|
||||
skip_soft_wrap: true,
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.assert_editor_state(indoc!(
|
||||
r#"fn fˇoo() {
|
||||
aaaa
|
||||
bbbb
|
||||
}
|
||||
oneˇ
|
||||
two"#
|
||||
));
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_add_selection_above_below(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
||||
@@ -7,7 +7,7 @@ use std::{
|
||||
use gpui::Pixels;
|
||||
use itertools::{Either, Itertools as _};
|
||||
use language::{Bias, Point, Selection, SelectionGoal};
|
||||
use multi_buffer::{MultiBufferDimension, MultiBufferOffset, ToPoint};
|
||||
use multi_buffer::{MultiBufferDimension, MultiBufferOffset, MultiBufferRow, ToPoint};
|
||||
use util::post_inc;
|
||||
|
||||
use crate::{
|
||||
@@ -405,7 +405,7 @@ impl SelectionsCollection {
|
||||
/// Returns `None` if the range is not empty but it starts past the line's
|
||||
/// length, meaning that the line isn't long enough to be contained within
|
||||
/// part of the provided range.
|
||||
pub fn build_columnar_selection(
|
||||
pub(crate) fn build_columnar_selection(
|
||||
&mut self,
|
||||
display_map: &DisplaySnapshot,
|
||||
row: DisplayRow,
|
||||
@@ -444,31 +444,33 @@ impl SelectionsCollection {
|
||||
}
|
||||
|
||||
/// Attempts to build a selection in the provided buffer row using the
|
||||
/// same UTF-16 column range as specified.
|
||||
/// same tab-expanded column range as specified.
|
||||
/// Returns `None` if the range is not empty but it starts past the line's
|
||||
/// length, meaning that the line isn't long enough to be contained within
|
||||
/// part of the provided range.
|
||||
fn build_columnar_selection_from_tab_expanded_columns(
|
||||
&mut self,
|
||||
display_map: &DisplaySnapshot,
|
||||
buffer_row: u32,
|
||||
positions: &Range<u32>,
|
||||
multi_buffer_row: MultiBufferRow,
|
||||
goal_columns: &Range<u32>,
|
||||
reversed: bool,
|
||||
text_layout_details: &TextLayoutDetails,
|
||||
) -> Option<Selection<Point>> {
|
||||
let is_empty = positions.start == positions.end;
|
||||
let line_len = display_map.tab_expanded_line_len(buffer_row);
|
||||
let is_empty = goal_columns.start == goal_columns.end;
|
||||
let line_len = display_map.tab_expanded_line_len(multi_buffer_row);
|
||||
|
||||
let (start, end) = if is_empty {
|
||||
let point = display_map.point_for_tab_expanded_column(buffer_row, positions.start);
|
||||
let point =
|
||||
display_map.point_for_tab_expanded_column(multi_buffer_row, goal_columns.start);
|
||||
(point, point)
|
||||
} else {
|
||||
if positions.start >= line_len {
|
||||
if goal_columns.start >= line_len {
|
||||
return None;
|
||||
}
|
||||
|
||||
let start = display_map.point_for_tab_expanded_column(buffer_row, positions.start);
|
||||
let end = display_map.point_for_tab_expanded_column(buffer_row, positions.end);
|
||||
let start =
|
||||
display_map.point_for_tab_expanded_column(multi_buffer_row, goal_columns.start);
|
||||
let end = display_map.point_for_tab_expanded_column(multi_buffer_row, goal_columns.end);
|
||||
(start, end)
|
||||
};
|
||||
|
||||
@@ -491,7 +493,7 @@ impl SelectionsCollection {
|
||||
|
||||
/// Finds the next columnar selection by walking display rows one at a time
|
||||
/// so that soft-wrapped lines are considered and not skipped.
|
||||
pub fn find_next_columnar_selection_by_display_row(
|
||||
pub(crate) fn find_next_columnar_selection_by_display_row(
|
||||
&mut self,
|
||||
display_map: &DisplaySnapshot,
|
||||
start_row: DisplayRow,
|
||||
@@ -524,7 +526,7 @@ impl SelectionsCollection {
|
||||
|
||||
/// Finds the next columnar selection by skipping to the next buffer row,
|
||||
/// ignoring soft-wrapped lines.
|
||||
pub fn find_next_columnar_selection_by_buffer_row(
|
||||
pub(crate) fn find_next_columnar_selection_by_buffer_row(
|
||||
&mut self,
|
||||
display_map: &DisplaySnapshot,
|
||||
start_row: DisplayRow,
|
||||
@@ -540,7 +542,7 @@ impl SelectionsCollection {
|
||||
let new_row =
|
||||
display_map.start_of_relative_buffer_row(DisplayPoint::new(row, 0), direction);
|
||||
row = new_row.row();
|
||||
let buffer_row = new_row.to_point(display_map).row;
|
||||
let buffer_row = MultiBufferRow(new_row.to_point(display_map).row);
|
||||
|
||||
if let Some(selection) = self.build_columnar_selection_from_tab_expanded_columns(
|
||||
display_map,
|
||||
|
||||
Reference in new issue
Block a user