Properly store editor restoration data (#28296)

We cannot compare versions and anchors between different `Buffer`s with
different `BufferId`s.

Release Notes:

- Fixed Zed panicking on editor reopen

Co-authored-by: Conrad Irwin <conrad@zed.dev>

crates/editor/src/editor.rs

@@ -1607,17 +1607,25 @@ impl Editor {
         }
         this.tasks_update_task = Some(this.refresh_runnables(window, cx));
         this._subscriptions.extend(project_subscriptions);
-        this._subscriptions
+
-            .push(cx.subscribe_self(|editor, e: &EditorEvent, cx| {
+        this._subscriptions.push(cx.subscribe_in(
+            &cx.entity(),
+            window,
+            |editor, _, e: &EditorEvent, window, cx| {
                 if let EditorEvent::SelectionsChanged { local } = e {
                     if *local {
                         let new_anchor = editor.scroll_manager.anchor();
+                        let snapshot = editor.snapshot(window, cx);
                         editor.update_restoration_data(cx, move |data| {
-                            data.scroll_anchor = new_anchor;
+                            data.scroll_position = (
+                                new_anchor.top_row(&snapshot.buffer_snapshot),
+                                new_anchor.offset,
+                            );
                         });
                     }
                 }
-            }));
+            },
+        ));
 
         this.end_selection(window, cx);
         this.scroll_manager.show_scrollbars(window, cx);
@@ -2367,22 +2375,30 @@ impl Editor {
         if selections.len() == 1 {
             cx.emit(SearchEvent::ActiveMatchChanged)
         }
-        if local && self.is_singleton(cx) {
+        if local {
-            let inmemory_selections = selections.iter().map(|s| s.range()).collect();
+            if let Some((_, _, buffer_snapshot)) = buffer.as_singleton() {
-            self.update_restoration_data(cx, |data| {
+                let inmemory_selections = selections
-                data.selections = inmemory_selections;
+                    .iter()
-            });
+                    .map(|s| {
+                        text::ToPoint::to_point(&s.range().start.text_anchor, buffer_snapshot)
+                            ..text::ToPoint::to_point(&s.range().end.text_anchor, buffer_snapshot)
+                    })
+                    .collect();
+                self.update_restoration_data(cx, |data| {
+                    data.selections = inmemory_selections;
+                });
 
-            if WorkspaceSettings::get(None, cx).restore_on_startup != RestoreOnStartupBehavior::None
+                if WorkspaceSettings::get(None, cx).restore_on_startup
-            {
+                    != RestoreOnStartupBehavior::None
-                if let Some(workspace_id) =
-                    self.workspace.as_ref().and_then(|workspace| workspace.1)
                 {
-                    let snapshot = self.buffer().read(cx).snapshot(cx);
+                    if let Some(workspace_id) =
-                    let selections = selections.clone();
+                        self.workspace.as_ref().and_then(|workspace| workspace.1)
-                    let background_executor = cx.background_executor().clone();
+                    {
-                    let editor_id = cx.entity().entity_id().as_u64() as ItemId;
+                        let snapshot = self.buffer().read(cx).snapshot(cx);
-                    self.serialize_selections = cx.background_spawn(async move {
+                        let selections = selections.clone();
+                        let background_executor = cx.background_executor().clone();
+                        let editor_id = cx.entity().entity_id().as_u64() as ItemId;
+                        self.serialize_selections = cx.background_spawn(async move {
                     background_executor.timer(SERIALIZATION_THROTTLE_TIME).await;
                     let db_selections = selections
                         .iter()
@@ -2399,6 +2415,7 @@ impl Editor {
                         .with_context(|| format!("persisting editor selections for editor {editor_id}, workspace {workspace_id:?}"))
                         .log_err();
                 });
+                    }
                 }
             }
         }
@@ -2407,18 +2424,27 @@ impl Editor {
     }
 
     fn folds_did_change(&mut self, cx: &mut Context<Self>) {
-        if !self.is_singleton(cx)
+        use text::ToOffset as _;
-            || WorkspaceSettings::get(None, cx).restore_on_startup == RestoreOnStartupBehavior::None
+        use text::ToPoint as _;
-        {
+
+        if WorkspaceSettings::get(None, cx).restore_on_startup == RestoreOnStartupBehavior::None {
             return;
         }
 
-        let snapshot = self.buffer().read(cx).snapshot(cx);
+        let Some(singleton) = self.buffer().read(cx).as_singleton() else {
+            return;
+        };
+
+        let snapshot = singleton.read(cx).snapshot();
         let inmemory_folds = self.display_map.update(cx, |display_map, cx| {
-            display_map
+            let display_snapshot = display_map.snapshot(cx);
-                .snapshot(cx)
+
-                .folds_in_range(0..snapshot.len())
+            display_snapshot
-                .map(|fold| fold.range.deref().clone())
+                .folds_in_range(0..display_snapshot.buffer_snapshot.len())
+                .map(|fold| {
+                    fold.range.start.text_anchor.to_point(&snapshot)
+                        ..fold.range.end.text_anchor.to_point(&snapshot)
+                })
                 .collect()
         });
         self.update_restoration_data(cx, |data| {
@@ -2436,8 +2462,8 @@ impl Editor {
                 .folds_in_range(0..snapshot.len())
                 .map(|fold| {
                     (
-                        fold.range.start.to_offset(&snapshot),
+                        fold.range.start.text_anchor.to_offset(&snapshot),
-                        fold.range.end.to_offset(&snapshot),
+                        fold.range.end.text_anchor.to_offset(&snapshot),
                     )
                 })
                 .collect()

crates/editor/src/items.rs

@@ -6,7 +6,6 @@ use crate::{
     scroll::ScrollAnchor,
 };
 use anyhow::{Context as _, Result, anyhow};
-use clock::Global;
 use collections::{HashMap, HashSet};
 use file_icons::FileIcons;
 use futures::future::try_join_all;
@@ -16,12 +15,12 @@ use gpui::{
     ParentElement, Pixels, SharedString, Styled, Task, WeakEntity, Window, point,
 };
 use language::{
-    Bias, Buffer, CharKind, DiskState, Point, SelectionGoal,
+    Bias, Buffer, BufferRow, CharKind, DiskState, LocalFile, Point, SelectionGoal,
     proto::serialize_anchor as serialize_text_anchor,
 };
 use lsp::DiagnosticSeverity;
 use project::{
-    Project, ProjectEntryId, ProjectItem as _, ProjectPath, lsp_store::FormatTrigger,
+    Project, ProjectItem as _, ProjectPath, lsp_store::FormatTrigger,
     project_settings::ProjectSettings, search::SearchQuery,
 };
 use rpc::proto::{self, PeerId, update_view};
@@ -30,13 +29,12 @@ use std::{
     any::TypeId,
     borrow::Cow,
     cmp::{self, Ordering},
-    collections::hash_map,
     iter,
     ops::Range,
-    path::Path,
+    path::{Path, PathBuf},
     sync::Arc,
 };
-use text::{BufferId, Selection};
+use text::{BufferId, BufferSnapshot, Selection};
 use theme::{Theme, ThemeSettings};
 use ui::{IconDecorationKind, prelude::*};
 use util::{ResultExt, TryFutureExt, paths::PathExt};
@@ -1243,26 +1241,14 @@ impl SerializableItem for Editor {
 
 #[derive(Debug, Default)]
 struct EditorRestorationData {
-    entries: HashMap<ProjectEntryId, RestorationData>,
+    entries: HashMap<PathBuf, RestorationData>,
 }
 
-#[derive(Debug)]
+#[derive(Default, Debug)]
 pub struct RestorationData {
-    pub scroll_anchor: ScrollAnchor,
+    pub scroll_position: (BufferRow, gpui::Point<f32>),
-    pub folds: Vec<Range<Anchor>>,
+    pub folds: Vec<Range<Point>>,
-    pub selections: Vec<Range<Anchor>>,
+    pub selections: Vec<Range<Point>>,
-    pub buffer_version: Global,
-}
-
-impl Default for RestorationData {
-    fn default() -> Self {
-        Self {
-            scroll_anchor: ScrollAnchor::new(),
-            folds: Vec::new(),
-            selections: Vec::new(),
-            buffer_version: Global::default(),
-        }
-    }
 }
 
 impl ProjectItem for Editor {
@@ -1280,21 +1266,37 @@ impl ProjectItem for Editor {
         cx: &mut Context<Self>,
     ) -> Self {
         let mut editor = Self::for_buffer(buffer.clone(), Some(project), window, cx);
-
+        if let Some((excerpt_id, buffer_id, snapshot)) =
-        if WorkspaceSettings::get(None, cx).restore_on_file_reopen {
+            editor.buffer().read(cx).snapshot(cx).as_singleton()
-            if let Some(restoration_data) = Self::project_item_kind()
+        {
-                .and_then(|kind| pane.project_item_restoration_data.get(&kind))
+            if WorkspaceSettings::get(None, cx).restore_on_file_reopen {
-                .and_then(|data| data.downcast_ref::<EditorRestorationData>())
+                if let Some(restoration_data) = Self::project_item_kind()
-                .and_then(|data| data.entries.get(&buffer.read(cx).entry_id(cx)?))
+                    .and_then(|kind| pane.project_item_restoration_data.get(&kind))
-                .filter(|data| !buffer.read(cx).version.changed_since(&data.buffer_version))
+                    .and_then(|data| data.downcast_ref::<EditorRestorationData>())
-            {
+                    .and_then(|data| {
-                editor.fold_ranges(restoration_data.folds.clone(), false, window, cx);
+                        let file = project::File::from_dyn(buffer.read(cx).file())?;
-                if !restoration_data.selections.is_empty() {
+                        data.entries.get(&file.abs_path(cx))
-                    editor.change_selections(None, window, cx, |s| {
+                    })
-                        s.select_ranges(restoration_data.selections.clone());
+                {
-                    });
+                    editor.fold_ranges(
+                        clip_ranges(&restoration_data.folds, &snapshot),
+                        false,
+                        window,
+                        cx,
+                    );
+                    if !restoration_data.selections.is_empty() {
+                        editor.change_selections(None, window, cx, |s| {
+                            s.select_ranges(clip_ranges(&restoration_data.selections, &snapshot));
+                        });
+                    }
+                    let (top_row, offset) = restoration_data.scroll_position;
+                    let anchor = Anchor::in_buffer(
+                        *excerpt_id,
+                        buffer_id,
+                        snapshot.anchor_before(Point::new(top_row, 0)),
+                    );
+                    editor.set_scroll_anchor(ScrollAnchor { anchor, offset }, window, cx);
                 }
-                editor.set_scroll_anchor(restoration_data.scroll_anchor, window, cx);
             }
         }
 
@@ -1302,6 +1304,19 @@ impl ProjectItem for Editor {
     }
 }
 
+fn clip_ranges<'a>(
+    original: impl IntoIterator<Item = &'a Range<Point>> + 'a,
+    snapshot: &'a BufferSnapshot,
+) -> Vec<Range<Point>> {
+    original
+        .into_iter()
+        .map(|range| {
+            snapshot.clip_point(range.start, Bias::Left)
+                ..snapshot.clip_point(range.end, Bias::Right)
+        })
+        .collect()
+}
+
 impl EventEmitter<SearchEvent> for Editor {}
 
 impl Editor {
@@ -1320,8 +1335,7 @@ impl Editor {
                 let kind = Editor::project_item_kind()?;
                 let pane = editor.workspace()?.read(cx).pane_for(&cx.entity())?;
                 let buffer = editor.buffer().read(cx).as_singleton()?;
-                let entry_id = buffer.read(cx).entry_id(cx)?;
+                let file_abs_path = project::File::from_dyn(buffer.read(cx).file())?.abs_path(cx);
-                let buffer_version = buffer.read(cx).version();
                 pane.update(cx, |pane, _| {
                     let data = pane
                         .project_item_restoration_data
@@ -1336,17 +1350,8 @@ impl Editor {
                         }
                     };
 
-                    let data = match data.entries.entry(entry_id) {
+                    let data = data.entries.entry(file_abs_path).or_default();
-                        hash_map::Entry::Occupied(o) => {
-                            if buffer_version.changed_since(&o.get().buffer_version) {
-                                return None;
-                            }
-                            o.into_mut()
-                        }
-                        hash_map::Entry::Vacant(v) => v.insert(RestorationData::default()),
-                    };
                     write(data);
-                    data.buffer_version = buffer_version;
                     Some(())
                 })
             });

crates/worktree/src/worktree.rs

@@ -3284,11 +3284,11 @@ impl language::File for File {
 
 impl language::LocalFile for File {
     fn abs_path(&self, cx: &App) -> PathBuf {
-        let worktree_path = &self.worktree.read(cx).as_local().unwrap().abs_path;
+        let worktree_path = &self.worktree.read(cx).abs_path();
         if self.path.as_ref() == Path::new("") {
-            worktree_path.as_path().to_path_buf()
+            worktree_path.to_path_buf()
         } else {
-            worktree_path.as_path().join(&self.path)
+            worktree_path.join(&self.path)
         }
     }