Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix sticky header in last buffer of a multibuffer (#26944)

Browse source 
This also simplifies our code to stop generating a last excerpt boundary
that we always ignore.

Closes #ISSUE

Release Notes:

- N/A
This commit is contained in:
Conrad Irwin 2025-03-17 12:39:57 -06:00 • committed by GitHub
parent 94b63808e0
commit 25772b8777
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 138 additions and 236 deletions
Download patch file Download diff file Expand all files Collapse all files

201
crates/editor/src/display_map/block_map.rs
View file

@ -240,7 +240,7 @@ pub struct BlockContext<'a, 'b> {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, Hash)]
pub enum BlockId {
ExcerptBoundary(Option<ExcerptId>),
ExcerptBoundary(ExcerptId),
FoldedBuffer(ExcerptId),
Custom(CustomBlockId),
}
@ -249,10 +249,9 @@ impl From<BlockId> for ElementId {
fn from(value: BlockId) -> Self {
match value {
BlockId::Custom(CustomBlockId(id)) => ("Block", id).into(),
BlockId::ExcerptBoundary(next_excerpt) => match next_excerpt {
Some(id) => ("ExcerptBoundary", EntityId::from(id)).into(),
None => "LastExcerptBoundary".into(),
},
BlockId::ExcerptBoundary(excerpt_id) => {
("ExcerptBoundary", EntityId::from(excerpt_id)).into()
}
BlockId::FoldedBuffer(id) => ("FoldedBuffer", EntityId::from(id)).into(),
}
}
@ -280,12 +279,10 @@ pub enum Block {
Custom(Arc<CustomBlock>),
FoldedBuffer {
first_excerpt: ExcerptInfo,
prev_excerpt: Option<ExcerptInfo>,
height: u32,
},
ExcerptBoundary {
prev_excerpt: Option<ExcerptInfo>,
next_excerpt: Option<ExcerptInfo>,
excerpt: ExcerptInfo,
height: u32,
starts_new_buffer: bool,
},
@ -295,9 +292,10 @@ impl Block {
pub fn id(&self) -> BlockId {
match self {
Block::Custom(block) => BlockId::Custom(block.id),
Block::ExcerptBoundary { next_excerpt, .. } => {
BlockId::ExcerptBoundary(next_excerpt.as_ref().map(|info| info.id))
}
Block::ExcerptBoundary {
excerpt: next_excerpt,
..
} => BlockId::ExcerptBoundary(next_excerpt.id),
Block::FoldedBuffer { first_excerpt, .. } => BlockId::FoldedBuffer(first_excerpt.id),
}
}
@ -320,7 +318,7 @@ impl Block {
match self {
Block::Custom(block) => matches!(block.placement, BlockPlacement::Above(_)),
Block::FoldedBuffer { .. } => false,
Block::ExcerptBoundary { next_excerpt, .. } => next_excerpt.is_some(),
Block::ExcerptBoundary { .. } => true,
}
}
@ -328,7 +326,7 @@ impl Block {
match self {
Block::Custom(block) => matches!(block.placement, BlockPlacement::Below(_)),
Block::FoldedBuffer { .. } => false,
Block::ExcerptBoundary { next_excerpt, .. } => next_excerpt.is_none(),
Block::ExcerptBoundary { .. } => false,
}
}
@ -347,6 +345,16 @@ impl Block {
Block::ExcerptBoundary { .. } => true,
}
}
pub fn is_buffer_header(&self) -> bool {
match self {
Block::Custom(_) => false,
Block::FoldedBuffer { .. } => true,
Block::ExcerptBoundary {
starts_new_buffer, ..
} => *starts_new_buffer,
}
}
}
impl Debug for Block {
@ -355,24 +363,21 @@ impl Debug for Block {
Self::Custom(block) => f.debug_struct("Custom").field("block", block).finish(),
Self::FoldedBuffer {
first_excerpt,
prev_excerpt,
height,
} => f
.debug_struct("FoldedBuffer")
.field("first_excerpt", &first_excerpt)
.field("prev_excerpt", prev_excerpt)
.field("height", height)
.finish(),
Self::ExcerptBoundary {
starts_new_buffer,
next_excerpt,
prev_excerpt,
..
excerpt,
height,
} => f
.debug_struct("ExcerptBoundary")
.field("prev_excerpt", prev_excerpt)
.field("next_excerpt", next_excerpt)
.field("excerpt", excerpt)
.field("starts_new_buffer", starts_new_buffer)
.field("height", height)
.finish(),
}
}
@ -724,23 +729,13 @@ impl BlockMap {
std::iter::from_fn(move || {
let excerpt_boundary = boundaries.next()?;
let wrap_row = if excerpt_boundary.next.is_some() {
wrap_snapshot.make_wrap_point(Point::new(excerpt_boundary.row.0, 0), Bias::Left)
} else {
wrap_snapshot.make_wrap_point(
Point::new(
excerpt_boundary.row.0,
buffer.line_len(excerpt_boundary.row),
),
Bias::Left,
)
}
.row();
let wrap_row = wrap_snapshot
.make_wrap_point(Point::new(excerpt_boundary.row.0, 0), Bias::Left)
.row();
let new_buffer_id = match (&excerpt_boundary.prev, &excerpt_boundary.next) {
(_, None) => None,
(None, Some(next)) => Some(next.buffer_id),
(Some(prev), Some(next)) => {
(None, next) => Some(next.buffer_id),
(Some(prev), next) => {
if prev.buffer_id != next.buffer_id {
Some(next.buffer_id)
} else {
@ -749,24 +744,18 @@ impl BlockMap {
}
};
let prev_excerpt = excerpt_boundary
.prev
.filter(|prev| !folded_buffers.contains(&prev.buffer_id));
let mut height = 0;
if let Some(new_buffer_id) = new_buffer_id {
let first_excerpt = excerpt_boundary.next.clone().unwrap();
let first_excerpt = excerpt_boundary.next.clone();
if folded_buffers.contains(&new_buffer_id) {
let mut last_excerpt_end_row = first_excerpt.end_row;
while let Some(next_boundary) = boundaries.peek() {
if let Some(next_excerpt_boundary) = &next_boundary.next {
if next_excerpt_boundary.buffer_id == new_buffer_id {
last_excerpt_end_row = next_excerpt_boundary.end_row;
} else {
break;
}
if next_boundary.next.buffer_id == new_buffer_id {
last_excerpt_end_row = next_boundary.next.end_row;
} else {
break;
}
boundaries.next();
@ -785,7 +774,6 @@ impl BlockMap {
return Some((
BlockPlacement::Replace(WrapRow(wrap_row)..=WrapRow(wrap_end_row)),
Block::FoldedBuffer {
prev_excerpt,
height: height + buffer_header_height,
first_excerpt,
},
@ -793,27 +781,16 @@ impl BlockMap {
}
}
if excerpt_boundary.next.is_some() {
if new_buffer_id.is_some() {
height += buffer_header_height;
} else {
height += excerpt_header_height;
}
}
if height == 0 {
return None;
if new_buffer_id.is_some() {
height += buffer_header_height;
} else {
height += excerpt_header_height;
}
Some((
if excerpt_boundary.next.is_some() {
BlockPlacement::Above(WrapRow(wrap_row))
} else {
BlockPlacement::Below(WrapRow(wrap_row))
},
BlockPlacement::Above(WrapRow(wrap_row)),
Block::ExcerptBoundary {
prev_excerpt,
next_excerpt: excerpt_boundary.next,
excerpt: excerpt_boundary.next,
height,
starts_new_buffer: new_buffer_id.is_some(),
},
@ -861,31 +838,14 @@ impl BlockMap {
placement_comparison.then_with(|| match (block_a, block_b) {
(
Block::ExcerptBoundary {
next_excerpt: next_excerpt_a,
..
excerpt: excerpt_a, ..
},
Block::ExcerptBoundary {
next_excerpt: next_excerpt_b,
..
excerpt: excerpt_b, ..
},
) => next_excerpt_a
.as_ref()
.map(|excerpt| excerpt.id)
.cmp(&next_excerpt_b.as_ref().map(|excerpt| excerpt.id)),
(Block::ExcerptBoundary { next_excerpt, .. }, Block::Custom(_)) => {
if next_excerpt.is_some() {
Ordering::Less
} else {
Ordering::Greater
}
}
(Block::Custom(_), Block::ExcerptBoundary { next_excerpt, .. }) => {
if next_excerpt.is_some() {
Ordering::Greater
} else {
Ordering::Less
}
}
) => Some(excerpt_a.id).cmp(&Some(excerpt_b.id)),
(Block::ExcerptBoundary { .. }, Block::Custom(_)) => Ordering::Less,
(Block::Custom(_), Block::ExcerptBoundary { .. }) => Ordering::Greater,
(Block::Custom(block_a), Block::Custom(block_b)) => block_a
.priority
.cmp(&block_b.priority)
@ -1405,51 +1365,19 @@ impl BlockSnapshot {
pub fn sticky_header_excerpt(&self, position: f32) -> Option<StickyHeaderExcerpt<'_>> {
let top_row = position as u32;
let mut cursor = self.transforms.cursor::<BlockRow>(&());
cursor.seek(&BlockRow(top_row), Bias::Left, &());
cursor.seek(&BlockRow(top_row), Bias::Right, &());
while let Some(transform) = cursor.item() {
let start = cursor.start().0;
let end = cursor.end(&()).0;
match &transform.block {
Some(Block::ExcerptBoundary {
prev_excerpt,
next_excerpt,
starts_new_buffer,
..
}) => {
let matches_start = (start as f32) < position;
if matches_start && top_row <= end {
return next_excerpt.as_ref().map(|excerpt| StickyHeaderExcerpt {
next_buffer_row: None,
excerpt,
});
}
let next_buffer_row = if *starts_new_buffer { Some(end) } else { None };
return prev_excerpt.as_ref().map(|excerpt| StickyHeaderExcerpt {
excerpt,
next_buffer_row,
});
Some(Block::ExcerptBoundary { excerpt, .. }) => {
return Some(StickyHeaderExcerpt { excerpt })
}
Some(Block::FoldedBuffer {
prev_excerpt: Some(excerpt),
..
}) if top_row <= start => {
return Some(StickyHeaderExcerpt {
next_buffer_row: Some(end),
excerpt,
});
Some(block) if block.is_buffer_header() => return None,
_ => {
cursor.prev(&());
continue;
}
Some(Block::FoldedBuffer { .. }) | Some(Block::Custom(_)) | None => {}
}
// This is needed to iterate past None / FoldedBuffer / Custom blocks. For FoldedBuffer,
// if scrolled slightly past the header of a folded block, the next block is needed for
// the sticky header.
cursor.next(&());
}
None
@ -1463,14 +1391,9 @@ impl BlockSnapshot {
return Some(Block::Custom(custom_block.clone()));
}
BlockId::ExcerptBoundary(next_excerpt_id) => {
if let Some(next_excerpt_id) = next_excerpt_id {
let excerpt_range = buffer.range_for_excerpt(next_excerpt_id)?;
self.wrap_snapshot
.make_wrap_point(excerpt_range.start, Bias::Left)
} else {
self.wrap_snapshot
.make_wrap_point(buffer.max_point(), Bias::Left)
}
let excerpt_range = buffer.range_for_excerpt(next_excerpt_id)?;
self.wrap_snapshot
.make_wrap_point(excerpt_range.start, Bias::Left)
}
BlockId::FoldedBuffer(excerpt_id) => self
.wrap_snapshot
@ -1748,7 +1671,6 @@ impl BlockChunks<'_> {
pub struct StickyHeaderExcerpt<'a> {
pub excerpt: &'a ExcerptInfo,
pub next_buffer_row: Option<u32>,
}
impl<'a> Iterator for BlockChunks<'a> {
@ -2254,9 +2176,9 @@ mod tests {
assert_eq!(
blocks,
vec![
(0..1, BlockId::ExcerptBoundary(Some(excerpt_ids[0]))), // path, header
(3..4, BlockId::ExcerptBoundary(Some(excerpt_ids[1]))), // path, header
(6..7, BlockId::ExcerptBoundary(Some(excerpt_ids[2]))), // path, header
(0..1, BlockId::ExcerptBoundary(excerpt_ids[0])), // path, header
(3..4, BlockId::ExcerptBoundary(excerpt_ids[1])), // path, header
(6..7, BlockId::ExcerptBoundary(excerpt_ids[2])), // path, header
]
);
}
@ -2953,10 +2875,7 @@ mod tests {
.iter()
.filter(|(_, block)| {
match block {
Block::FoldedBuffer { prev_excerpt, .. } => {
assert!(prev_excerpt.is_none());
true
}
Block::FoldedBuffer { .. } => true,
_ => false,
}
})

80
crates/editor/src/element.rs
View file

@ -2640,7 +2640,7 @@ impl EditorElement {
}
Block::ExcerptBoundary {
next_excerpt,
excerpt,
height,
starts_new_buffer,
..
@ -2648,40 +2648,31 @@ impl EditorElement {
let color = cx.theme().colors().clone();
let mut result = v_flex().id(block_id).w_full();
if let Some(next_excerpt) = next_excerpt {
let jump_data =
header_jump_data(snapshot, block_row_start, *height, next_excerpt);
let jump_data = header_jump_data(snapshot, block_row_start, *height, excerpt);
if *starts_new_buffer {
if sticky_header_excerpt_id != Some(next_excerpt.id) {
let selected = selected_buffer_ids.contains(&next_excerpt.buffer_id);
if *starts_new_buffer {
if sticky_header_excerpt_id != Some(excerpt.id) {
let selected = selected_buffer_ids.contains(&excerpt.buffer_id);
result = result.child(self.render_buffer_header(
next_excerpt,
false,
selected,
false,
jump_data,
window,
cx,
));
} else {
result = result
.child(div().h(FILE_HEADER_HEIGHT as f32 * window.line_height()));
}
result = result.child(self.render_buffer_header(
excerpt, false, selected, false, jump_data, window, cx,
));
} else {
result = result.child(
h_flex().relative().child(
div()
.top(line_height / 2.)
.absolute()
.w_full()
.h_px()
.bg(color.border_variant),
),
);
};
}
result =
result.child(div().h(FILE_HEADER_HEIGHT as f32 * window.line_height()));
}
} else {
result = result.child(
h_flex().relative().child(
div()
.top(line_height / 2.)
.absolute()
.w_full()
.h_px()
.bg(color.border_variant),
),
);
};
result.into_any()
}
@ -2972,6 +2963,7 @@ impl EditorElement {
element,
available_space: size(AvailableSpace::MinContent, element_size.height.into()),
style: BlockStyle::Fixed,
is_buffer_header: block.is_buffer_header(),
});
}
@ -3022,6 +3014,7 @@ impl EditorElement {
element,
available_space: size(width.into(), element_size.height.into()),
style,
is_buffer_header: block.is_buffer_header(),
});
}
@ -3072,6 +3065,7 @@ impl EditorElement {
element,
available_space: size(width, element_size.height.into()),
style,
is_buffer_header: block.is_buffer_header(),
});
}
}
@ -3133,15 +3127,13 @@ impl EditorElement {
fn layout_sticky_buffer_header(
&self,
StickyHeaderExcerpt {
excerpt,
next_buffer_row,
}: StickyHeaderExcerpt<'_>,
StickyHeaderExcerpt { excerpt }: StickyHeaderExcerpt<'_>,
scroll_position: f32,
line_height: Pixels,
snapshot: &EditorSnapshot,
hitbox: &Hitbox,
selected_buffer_ids: &Vec<BufferId>,
blocks: &[BlockLayout],
window: &mut Window,
cx: &mut App,
) -> AnyElement {
@ -3177,17 +3169,23 @@ impl EditorElement {
.into_any_element();
let mut origin = hitbox.origin;
// Move floating header up to avoid colliding with the next buffer header.
for block in blocks.iter() {
if !block.is_buffer_header {
continue;
}
if let Some(next_buffer_row) = next_buffer_row {
// Push up the sticky header when the excerpt is getting close to the top of the viewport
let max_row = next_buffer_row - FILE_HEADER_HEIGHT * 2;
let Some(display_row) = block.row.filter(|row| row.0 > scroll_position as u32) else {
continue;
};
let max_row = display_row.0.saturating_sub(FILE_HEADER_HEIGHT);
let offset = scroll_position - max_row as f32;
if offset > 0.0 {
origin.y -= Pixels(offset) * line_height;
}
break;
}
let size = size(
@ -7043,6 +7041,7 @@ impl Element for EditorElement {
&snapshot,
&hitbox,
&selected_buffer_ids,
&blocks,
window,
cx,
)
@ -7926,6 +7925,7 @@ struct BlockLayout {
element: AnyElement,
available_space: Size<AvailableSpace>,
style: BlockStyle,
is_buffer_header: bool,
}
pub fn layout_line(

20
crates/editor/src/movement.rs
View file

@ -1109,14 +1109,14 @@ mod tests {
px(14.0),
None,
0,
2,
1,
FoldPlaceholder::test(),
cx,
)
});
let snapshot = display_map.update(cx, |map, cx| map.snapshot(cx));
assert_eq!(snapshot.text(), "abc\ndefg\nhijkl\nmn");
assert_eq!(snapshot.text(), "abc\ndefg\n\nhijkl\nmn");
let col_2_x = snapshot
.x_for_display_point(DisplayPoint::new(DisplayRow(0), 2), &text_layout_details);
@ -1181,13 +1181,13 @@ mod tests {
);
let col_5_x = snapshot
.x_for_display_point(DisplayPoint::new(DisplayRow(2), 5), &text_layout_details);
.x_for_display_point(DisplayPoint::new(DisplayRow(3), 5), &text_layout_details);
// Move up and down across second excerpt's header
assert_eq!(
up(
&snapshot,
DisplayPoint::new(DisplayRow(2), 5),
DisplayPoint::new(DisplayRow(3), 5),
SelectionGoal::HorizontalPosition(col_5_x.0),
false,
&text_layout_details
@ -1206,38 +1206,38 @@ mod tests {
&text_layout_details
),
(
DisplayPoint::new(DisplayRow(2), 5),
DisplayPoint::new(DisplayRow(3), 5),
SelectionGoal::HorizontalPosition(col_5_x.0)
),
);
let max_point_x = snapshot
.x_for_display_point(DisplayPoint::new(DisplayRow(3), 2), &text_layout_details);
.x_for_display_point(DisplayPoint::new(DisplayRow(4), 2), &text_layout_details);
// Can't move down off the end, and attempting to do so leaves the selection goal unchanged
assert_eq!(
down(
&snapshot,
DisplayPoint::new(DisplayRow(3), 0),
DisplayPoint::new(DisplayRow(4), 0),
SelectionGoal::HorizontalPosition(0.0),
false,
&text_layout_details
),
(
DisplayPoint::new(DisplayRow(3), 2),
DisplayPoint::new(DisplayRow(4), 2),
SelectionGoal::HorizontalPosition(0.0)
),
);
assert_eq!(
down(
&snapshot,
DisplayPoint::new(DisplayRow(3), 2),
DisplayPoint::new(DisplayRow(4), 2),
SelectionGoal::HorizontalPosition(max_point_x.0),
false,
&text_layout_details
),
(
DisplayPoint::new(DisplayRow(3), 2),
DisplayPoint::new(DisplayRow(4), 2),
SelectionGoal::HorizontalPosition(max_point_x.0)
),
);

51
crates/multi_buffer/src/multi_buffer.rs
View file

@ -346,17 +346,16 @@ impl std::fmt::Debug for ExcerptInfo {
#[derive(Debug)]
pub struct ExcerptBoundary {
pub prev: Option<ExcerptInfo>,
pub next: Option<ExcerptInfo>,
pub next: ExcerptInfo,
/// The row in the `MultiBuffer` where the boundary is located
pub row: MultiBufferRow,
}
impl ExcerptBoundary {
pub fn starts_new_buffer(&self) -> bool {
match (self.prev.as_ref(), self.next.as_ref()) {
match (self.prev.as_ref(), &self.next) {
(None, _) => true,
(Some(_), None) => false,
(Some(prev), Some(next)) => prev.buffer_id != next.buffer_id,
(Some(prev), next) => prev.buffer_id != next.buffer_id,
}
}
}
@ -5200,27 +5199,19 @@ impl MultiBufferSnapshot {
cursor.next_excerpt();
let mut visited_end = false;
iter::from_fn(move || loop {
if self.singleton {
return None;
}
let next_region = cursor.region();
let next_region = cursor.region()?;
cursor.next_excerpt();
if !bounds.contains(&next_region.range.start.key) {
prev_region = Some(next_region);
continue;
}
let next_region_start = if let Some(region) = &next_region {
if !bounds.contains(&region.range.start.key) {
prev_region = next_region;
continue;
}
region.range.start.value.unwrap()
} else {
if !bounds.contains(&self.len()) {
return None;
}
self.max_point()
};
let next_region_start = next_region.range.start.value.unwrap();
let next_region_end = if let Some(region) = cursor.region() {
region.range.start.value.unwrap()
} else {
@ -5235,29 +5226,21 @@ impl MultiBufferSnapshot {
end_row: MultiBufferRow(next_region_start.row),
});
let next = next_region.as_ref().map(|region| ExcerptInfo {
id: region.excerpt.id,
buffer: region.excerpt.buffer.clone(),
buffer_id: region.excerpt.buffer_id,
range: region.excerpt.range.clone(),
end_row: if region.excerpt.has_trailing_newline {
let next = ExcerptInfo {
id: next_region.excerpt.id,
buffer: next_region.excerpt.buffer.clone(),
buffer_id: next_region.excerpt.buffer_id,
range: next_region.excerpt.range.clone(),
end_row: if next_region.excerpt.has_trailing_newline {
MultiBufferRow(next_region_end.row - 1)
} else {
MultiBufferRow(next_region_end.row)
},
});
if next.is_none() {
if visited_end {
return None;
} else {
visited_end = true;
}
}
};
let row = MultiBufferRow(next_region_start.row);
prev_region = next_region;
prev_region = Some(next_region);
return Some(ExcerptBoundary { row, prev, next });
})

22
crates/multi_buffer/src/multi_buffer_tests.rs
View file

@ -341,17 +341,17 @@ fn test_excerpt_boundaries_and_clipping(cx: &mut App) {
) -> Vec<(MultiBufferRow, String, bool)> {
snapshot
.excerpt_boundaries_in_range(range)
.filter_map(|boundary| {
.map(|boundary| {
let starts_new_buffer = boundary.starts_new_buffer();
boundary.next.map(|next| {
(
boundary.row,
next.buffer
.text_for_range(next.range.context)
.collect::<String>(),
starts_new_buffer,
)
})
(
boundary.row,
boundary
.next
.buffer
.text_for_range(boundary.next.range.context)
.collect::<String>(),
starts_new_buffer,
)
})
.collect::<Vec<_>>()
}
@ -2695,7 +2695,7 @@ async fn test_random_multibuffer(cx: &mut TestAppContext, mut rng: StdRng) {
let actual_text = snapshot.text();
let actual_boundary_rows = snapshot
.excerpt_boundaries_in_range(0..)
.filter_map(|b| if b.next.is_some() { Some(b.row) } else { None })
.map(|b| b.row)
.collect::<HashSet<_>>();
let actual_row_infos = snapshot.row_infos(MultiBufferRow(0)).collect::<Vec<_>>();