oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

multi_buffer: Typed MultiBufferOffset (#42707)

Browse Source 
This PR introduces a new `MultiBufferOffset` new type wrapping size. The
goal of this is to make it clear at the type level when we are
interacting with offsets of a multi buffer versus offsets of a language
/ text buffer. This improves readability of things quite a bit by making
it clear what kind of offsets one is working with while also reducing
accidental bugs by using the wrong kin of offset for the wrong API.

This PR also uncovered two minor bugs due to that.

Does not yet introduce the MultiBufferPoint equivalent, that is for a
follow up PR.

Release Notes:

- N/A *or* Added/Fixed/Improved ...
This commit is contained in:
Lukas Wirth
2025-11-19 22:00:58 +00:00
committed by GitHub
parent 5e21457f21
commit c98b2d6944
81 changed files with 2957 additions and 1564 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/vim/src/helix/boundary.rs
+39 -42
View File
@@ -1,10 +1,7 @@
use std::{
cmp::Ordering,
ops::{Deref, DerefMut, Range},
};
use std::{cmp::Ordering, ops::Range};
use editor::{
DisplayPoint,
DisplayPoint, MultiBufferOffset,
display_map::{DisplaySnapshot, ToDisplayPoint},
movement,
};
@@ -104,8 +101,8 @@ trait BoundedObject {
let next_end = self.next_end(map, end_search_start, outer)?;
let maybe_next_start = self.next_start(map, start_search_start, outer);
if let Some(next_start) = maybe_next_start
&& (*next_start < *next_end
|| *next_start == *next_end && self.can_be_zero_width(outer))
&& (next_start.0 < next_end.0
|| next_start.0 == next_end.0 && self.can_be_zero_width(outer))
&& !self.ambiguous_outer()
{
let closing = self.close_at_end(next_start, map, outer)?;
@@ -133,8 +130,8 @@ trait BoundedObject {
let previous_start = self.previous_start(map, start_search_end, outer)?;
let maybe_previous_end = self.previous_end(map, end_search_end, outer);
if let Some(previous_end) = maybe_previous_end
&& (*previous_end > *previous_start
|| *previous_end == *previous_start && self.can_be_zero_width(outer))
&& (previous_end.0 > previous_start.0
|| previous_end.0 == previous_start.0 && self.can_be_zero_width(outer))
&& !self.ambiguous_outer()
{
let closing = self.close_at_start(previous_end, map, outer)?;
@@ -151,30 +148,22 @@ trait BoundedObject {
}
}
#[derive(Clone, Copy, PartialEq, Debug)]
struct Offset(usize);
impl Deref for Offset {
type Target = usize;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl DerefMut for Offset {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
#[derive(Clone, Copy, PartialEq, Debug, PartialOrd, Ord, Eq)]
struct Offset(MultiBufferOffset);
impl Offset {
fn next(self, map: &DisplaySnapshot) -> Option<Self> {
let next = Self(map.buffer_snapshot().clip_offset(*self + 1, Bias::Right));
(*next > *self).then(|| next)
let next = Self(
map.buffer_snapshot()
.clip_offset(self.0 + 1usize, Bias::Right),
);
(next.0 > self.0).then(|| next)
}
fn previous(self, map: &DisplaySnapshot) -> Option<Self> {
if *self == 0 {
if self.0 == MultiBufferOffset(0) {
return None;
}
Some(Self(
map.buffer_snapshot().clip_offset(*self - 1, Bias::Left),
map.buffer_snapshot().clip_offset(self.0 - 1, Bias::Left),
))
}
fn range(
@@ -211,7 +200,7 @@ impl<B: BoundedObject> HelixTextObject for B {
let max_end = self.close_at_end(search_start, map, find_outer)?;
let min_start = self.close_at_start(max_end, map, find_outer)?;
(*min_start <= *relative_to.start).then(|| min_start..max_end)
(min_start <= relative_to.start).then(|| min_start..max_end)
})
}
@@ -279,8 +268,8 @@ fn relative_range<B: BoundedObject>(
min_start..max_end
};
let start = wanted_range.start.clone().to_display_point(map);
let end = wanted_range.end.clone().to_display_point(map);
let start = wanted_range.start.0.to_display_point(map);
let end = wanted_range.end.0.to_display_point(map);
Some(start..end)
}
@@ -390,7 +379,7 @@ impl ImmediateBoundary {
impl BoundedObject for ImmediateBoundary {
fn next_start(&self, map: &DisplaySnapshot, from: Offset, outer: bool) -> Option<Offset> {
try_find_boundary(map, from, |left, right| {
let classifier = map.buffer_snapshot().char_classifier_at(*from);
let classifier = map.buffer_snapshot().char_classifier_at(from.0);
if outer {
self.is_outer_start(left, right, classifier)
} else {
@@ -400,7 +389,7 @@ impl BoundedObject for ImmediateBoundary {
}
fn next_end(&self, map: &DisplaySnapshot, from: Offset, outer: bool) -> Option<Offset> {
try_find_boundary(map, from, |left, right| {
let classifier = map.buffer_snapshot().char_classifier_at(*from);
let classifier = map.buffer_snapshot().char_classifier_at(from.0);
if outer {
self.is_outer_end(left, right, classifier)
} else {
@@ -410,7 +399,7 @@ impl BoundedObject for ImmediateBoundary {
}
fn previous_start(&self, map: &DisplaySnapshot, from: Offset, outer: bool) -> Option<Offset> {
try_find_preceding_boundary(map, from, |left, right| {
let classifier = map.buffer_snapshot().char_classifier_at(*from);
let classifier = map.buffer_snapshot().char_classifier_at(from.0);
if outer {
self.is_outer_start(left, right, classifier)
} else {
@@ -420,7 +409,7 @@ impl BoundedObject for ImmediateBoundary {
}
fn previous_end(&self, map: &DisplaySnapshot, from: Offset, outer: bool) -> Option<Offset> {
try_find_preceding_boundary(map, from, |left, right| {
let classifier = map.buffer_snapshot().char_classifier_at(*from);
let classifier = map.buffer_snapshot().char_classifier_at(from.0);
if outer {
self.is_outer_end(left, right, classifier)
} else {
@@ -572,7 +561,7 @@ impl FuzzyBoundary {
boundary_kind: Boundary,
) -> Option<Offset> {
let generate_boundary_data = |left, right, point: Offset| {
let classifier = map.buffer_snapshot().char_classifier_at(*from);
let classifier = map.buffer_snapshot().char_classifier_at(from.0);
let reach_boundary = if outer && boundary_kind == Boundary::Start {
self.is_near_potential_outer_start(left, right, &classifier)
} else if !outer && boundary_kind == Boundary::Start {
@@ -598,9 +587,9 @@ impl FuzzyBoundary {
Ordering::Greater => !backward,
});
if backward {
boundaries.max_by_key(|boundary| **boundary)
boundaries.max_by_key(|boundary| *boundary)
} else {
boundaries.min_by_key(|boundary| **boundary)
boundaries.min_by_key(|boundary| *boundary)
}
}
}
@@ -662,15 +651,15 @@ fn try_find_boundary_data<T>(
) -> Option<T> {
let mut prev_ch = map
.buffer_snapshot()
.reversed_chars_at(*from)
.reversed_chars_at(from.0)
.next()
.unwrap_or('\0');
for ch in map.buffer_snapshot().chars_at(*from).chain(['\0']) {
for ch in map.buffer_snapshot().chars_at(from.0).chain(['\0']) {
if let Some(boundary_information) = boundary_information(prev_ch, ch, from) {
return Some(boundary_information);
}
*from += ch.len_utf8();
from.0 += ch.len_utf8();
prev_ch = ch;
}
@@ -702,13 +691,21 @@ fn try_find_preceding_boundary_data<T>(
mut from: Offset,
is_boundary: impl Fn(char, char, Offset) -> Option<T>,
) -> Option<T> {
let mut prev_ch = map.buffer_snapshot().chars_at(*from).next().unwrap_or('\0');
let mut prev_ch = map
.buffer_snapshot()
.chars_at(from.0)
.next()
.unwrap_or('\0');
for ch in map.buffer_snapshot().reversed_chars_at(*from).chain(['\0']) {
for ch in map
.buffer_snapshot()
.reversed_chars_at(from.0)
.chain(['\0'])
{
if let Some(boundary_information) = is_boundary(ch, prev_ch, from) {
return Some(boundary_information);
}
from.0 = from.0.saturating_sub(ch.len_utf8());
from.0.0 = from.0.0.saturating_sub(ch.len_utf8());
prev_ch = ch;
}
crates/vim/src/helix/duplicate.rs
+2 -2
View File
@@ -1,6 +1,6 @@
use std::ops::Range;
use editor::{DisplayPoint, display_map::DisplaySnapshot};
use editor::{DisplayPoint, MultiBufferOffset, display_map::DisplaySnapshot};
use gpui::Context;
use text::Bias;
use ui::Window;
@@ -111,7 +111,7 @@ fn find_next_valid_duplicate_space(
fn display_point_range_to_offset_range(
range: &Range<DisplayPoint>,
map: &DisplaySnapshot,
) -> Range<usize> {
) -> Range<MultiBufferOffset> {
range.start.to_offset(map, Bias::Left)..range.end.to_offset(map, Bias::Right)
}
crates/vim/src/helix/paste.rs
+1 -1
View File
@@ -125,7 +125,7 @@ impl Vim {
s.select_ranges(new_selections.into_iter().map(|(anchor, len)| {
let offset = anchor.to_offset(&snapshot);
if action.before {
offset.saturating_sub(len)..offset
offset.saturating_sub_usize(len)..offset
} else {
offset..(offset + len)
}