Clip scroll_top_row before navigating back to it

Co-Authored-By: Nathan Sobo <nathan@zed.dev>
This commit is contained in:
Antonio Scandurra 2022-04-27 16:11:42 +02:00
parent 27e693d8f7
commit cde5a45318
8 changed files with 88 additions and 35 deletions

View file

@ -847,14 +847,15 @@ struct ClipboardSelection {
is_entire_line: bool,
}
#[derive(Debug)]
pub struct NavigationData {
// Matching offsets for anchor and scroll_top_anchor allows us to recreate the anchor if the buffer
// has since been closed
cursor_anchor: Anchor,
cursor_offset: usize,
cursor_point: Point,
scroll_position: Vector2F,
scroll_top_anchor: Anchor,
scroll_top_offset: usize,
scroll_top_row: u32,
}
pub struct EditorCreated(pub ViewHandle<Editor>);
@ -1112,6 +1113,7 @@ impl Editor {
self.scroll_top_anchor = anchor;
}
self.autoscroll_request.take();
cx.emit(Event::ScrollPositionChanged { local });
cx.notify();
}
@ -3905,9 +3907,8 @@ impl Editor {
) {
if let Some(nav_history) = &self.nav_history {
let buffer = self.buffer.read(cx).read(cx);
let offset = position.to_offset(&buffer);
let point = position.to_point(&buffer);
let scroll_top_offset = self.scroll_top_anchor.to_offset(&buffer);
let scroll_top_row = self.scroll_top_anchor.to_point(&buffer).row;
drop(buffer);
if let Some(new_position) = new_position {
@ -3919,10 +3920,10 @@ impl Editor {
nav_history.push(Some(NavigationData {
cursor_anchor: position,
cursor_offset: offset,
cursor_point: point,
scroll_position: self.scroll_position,
scroll_top_anchor: self.scroll_top_anchor.clone(),
scroll_top_offset,
scroll_top_row,
}));
}
}

View file

@ -1,4 +1,4 @@
use crate::{Anchor, Autoscroll, Editor, Event, ExcerptId, NavigationData, ToOffset, ToPoint as _};
use crate::{Anchor, Autoscroll, Editor, Event, ExcerptId, NavigationData, ToPoint as _};
use anyhow::{anyhow, Result};
use futures::FutureExt;
use gpui::{
@ -245,19 +245,21 @@ fn deserialize_selection(
impl Item for Editor {
fn navigate(&mut self, data: Box<dyn std::any::Any>, cx: &mut ViewContext<Self>) -> bool {
if let Some(data) = data.downcast_ref::<NavigationData>() {
if let Ok(data) = data.downcast::<NavigationData>() {
let buffer = self.buffer.read(cx).read(cx);
let offset = if buffer.can_resolve(&data.cursor_anchor) {
data.cursor_anchor.to_offset(&buffer)
data.cursor_anchor.to_point(&buffer)
} else {
buffer.clip_offset(data.cursor_offset, Bias::Left)
buffer.clip_point(data.cursor_point, Bias::Left)
};
let newest_selection = self.newest_selection_with_snapshot::<usize>(&buffer);
let newest_selection = self.newest_selection_with_snapshot::<Point>(&buffer);
let scroll_top_anchor = if buffer.can_resolve(&data.scroll_top_anchor) {
data.scroll_top_anchor.clone()
data.scroll_top_anchor
} else {
buffer.anchor_at(data.scroll_top_offset, Bias::Left)
buffer.anchor_before(
buffer.clip_point(Point::new(data.scroll_top_row, 0), Bias::Left),
)
};
drop(buffer);

View file

@ -147,6 +147,7 @@ pub fn serialize_anchor(anchor: &Anchor) -> proto::Anchor {
Bias::Left => proto::Bias::Left as i32,
Bias::Right => proto::Bias::Right as i32,
},
buffer_id: anchor.buffer_id,
}
}
@ -330,6 +331,7 @@ pub fn deserialize_anchor(anchor: proto::Anchor) -> Option<Anchor> {
proto::Bias::Left => Bias::Left,
proto::Bias::Right => Bias::Right,
},
buffer_id: anchor.buffer_id,
})
}

View file

@ -685,6 +685,7 @@ message Anchor {
uint32 local_timestamp = 2;
uint64 offset = 3;
Bias bias = 4;
optional uint64 buffer_id = 5;
}
enum Bias {

View file

@ -5,4 +5,4 @@ pub mod proto;
pub use conn::Connection;
pub use peer::*;
pub const PROTOCOL_VERSION: u32 = 14;
pub const PROTOCOL_VERSION: u32 = 15;

View file

@ -9,6 +9,7 @@ pub struct Anchor {
pub timestamp: clock::Local,
pub offset: usize,
pub bias: Bias,
pub buffer_id: Option<u64>,
}
impl Anchor {
@ -16,12 +17,14 @@ impl Anchor {
timestamp: clock::Local::MIN,
offset: usize::MIN,
bias: Bias::Left,
buffer_id: None,
};
pub const MAX: Self = Self {
timestamp: clock::Local::MAX,
offset: usize::MAX,
bias: Bias::Right,
buffer_id: None,
};
pub fn cmp(&self, other: &Anchor, buffer: &BufferSnapshot) -> Ordering {

View file

@ -50,7 +50,6 @@ pub struct Buffer {
deferred_ops: OperationQueue<Operation>,
deferred_replicas: HashSet<ReplicaId>,
replica_id: ReplicaId,
remote_id: u64,
local_clock: clock::Local,
pub lamport_clock: clock::Lamport,
subscriptions: Topic,
@ -61,6 +60,7 @@ pub struct Buffer {
#[derive(Clone, Debug)]
pub struct BufferSnapshot {
replica_id: ReplicaId,
remote_id: u64,
visible_text: Rope,
deleted_text: Rope,
undo_map: UndoMap,
@ -562,6 +562,7 @@ impl Buffer {
Buffer {
snapshot: BufferSnapshot {
replica_id,
remote_id,
visible_text,
deleted_text: Rope::new(),
fragments,
@ -573,7 +574,6 @@ impl Buffer {
deferred_ops: OperationQueue::new(),
deferred_replicas: HashSet::default(),
replica_id,
remote_id,
local_clock,
lamport_clock,
subscriptions: Default::default(),
@ -1795,12 +1795,15 @@ impl BufferSnapshot {
timestamp: fragment.insertion_timestamp.local(),
offset: fragment.insertion_offset + overshoot,
bias,
buffer_id: Some(self.remote_id),
}
}
}
pub fn can_resolve(&self, anchor: &Anchor) -> bool {
*anchor == Anchor::MIN || *anchor == Anchor::MAX || self.version.observed(anchor.timestamp)
*anchor == Anchor::MIN
|| *anchor == Anchor::MAX
|| (Some(self.remote_id) == anchor.buffer_id && self.version.observed(anchor.timestamp))
}
pub fn clip_offset(&self, offset: usize, bias: Bias) -> usize {

View file

@ -976,12 +976,27 @@ mod tests {
.unwrap()
.downcast::<Editor>()
.unwrap();
editor3
.update(cx, |editor, cx| {
editor.select_display_ranges(
&[DisplayPoint::new(12, 0)..DisplayPoint::new(12, 0)],
cx,
);
editor.newline(&Default::default(), cx);
editor.newline(&Default::default(), cx);
editor.move_down(&Default::default(), cx);
editor.move_down(&Default::default(), cx);
editor.save(params.project.clone(), cx)
})
.await
.unwrap();
editor3.update(cx, |editor, cx| {
editor.select_display_ranges(&[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)], cx);
editor.set_scroll_position(vec2f(0., 12.5), cx)
});
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(15, 0))
(file3.clone(), DisplayPoint::new(16, 0), 12.5)
);
workspace
@ -989,7 +1004,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(0, 0))
(file3.clone(), DisplayPoint::new(0, 0), 0.)
);
workspace
@ -997,7 +1012,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file2.clone(), DisplayPoint::new(0, 0))
(file2.clone(), DisplayPoint::new(0, 0), 0.)
);
workspace
@ -1005,7 +1020,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(10, 0))
(file1.clone(), DisplayPoint::new(10, 0), 0.)
);
workspace
@ -1013,7 +1028,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(0, 0))
(file1.clone(), DisplayPoint::new(0, 0), 0.)
);
// Go back one more time and ensure we don't navigate past the first item in the history.
@ -1022,7 +1037,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(0, 0))
(file1.clone(), DisplayPoint::new(0, 0), 0.)
);
workspace
@ -1030,7 +1045,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(10, 0))
(file1.clone(), DisplayPoint::new(10, 0), 0.)
);
workspace
@ -1038,7 +1053,7 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file2.clone(), DisplayPoint::new(0, 0))
(file2.clone(), DisplayPoint::new(0, 0), 0.)
);
// Go forward to an item that has been closed, ensuring it gets re-opened at the same
@ -1056,7 +1071,23 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(0, 0))
(file3.clone(), DisplayPoint::new(0, 0), 0.)
);
workspace
.update(cx, |w, cx| Pane::go_forward(w, None, cx))
.await;
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(16, 0), 12.5)
);
workspace
.update(cx, |w, cx| Pane::go_back(w, None, cx))
.await;
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(0, 0), 0.)
);
// Go back to an item that has been closed and removed from disk, ensuring it gets skipped.
@ -1079,17 +1110,18 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(10, 0))
(file1.clone(), DisplayPoint::new(10, 0), 0.)
);
workspace
.update(cx, |w, cx| Pane::go_forward(w, None, cx))
.await;
assert_eq!(
active_location(&workspace, cx),
(file3.clone(), DisplayPoint::new(0, 0))
(file3.clone(), DisplayPoint::new(0, 0), 0.)
);
// Modify file to remove nav history location, and ensure duplicates are skipped
// Modify file to collapse multiple nav history entries into the same location.
// Ensure we don't visit the same location twice when navigating.
editor1.update(cx, |editor, cx| {
editor.select_display_ranges(&[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)], cx)
});
@ -1125,25 +1157,34 @@ mod tests {
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(2, 0))
(file1.clone(), DisplayPoint::new(2, 0), 0.)
);
workspace
.update(cx, |w, cx| Pane::go_back(w, None, cx))
.await;
assert_eq!(
active_location(&workspace, cx),
(file1.clone(), DisplayPoint::new(3, 0))
(file1.clone(), DisplayPoint::new(3, 0), 0.)
);
fn active_location(
workspace: &ViewHandle<Workspace>,
cx: &mut TestAppContext,
) -> (ProjectPath, DisplayPoint) {
) -> (ProjectPath, DisplayPoint, f32) {
workspace.update(cx, |workspace, cx| {
let item = workspace.active_item(cx).unwrap();
let editor = item.downcast::<Editor>().unwrap();
let selections = editor.update(cx, |editor, cx| editor.selected_display_ranges(cx));
(item.project_path(cx).unwrap(), selections[0].start)
let (selections, scroll_position) = editor.update(cx, |editor, cx| {
(
editor.selected_display_ranges(cx),
editor.scroll_position(cx),
)
});
(
item.project_path(cx).unwrap(),
selections[0].start,
scroll_position.y(),
)
})
}
}