Drop refcell use, make view simply ref doc.id.

4 files changed, 74 insertions, 36 deletions

diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index f6c7c70d..0286b2b9 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -8,6 +8,8 @@ use helix_core::{
     ChangeSet, Diagnostic, History, Rope, Selection, State, Syntax, Transaction,
 };
 
+use crate::DocumentId;
+
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
 pub enum Mode {
     Normal,
@@ -18,6 +20,7 @@ pub enum Mode {
 
 pub struct Document {
     // rope + selection
+    pub(crate) id: DocumentId,
     state: State,
     path: Option<PathBuf>,
 
@@ -66,6 +69,7 @@ impl Document {
         let old_state = None;
 
         Self {
+            id: DocumentId::default(),
             path: None,
             state: State::new(text),
             mode: Mode::Normal,
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 08f4183d..802ca3e1 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -1,7 +1,7 @@
 use crate::{theme::Theme, tree::Tree, Document, DocumentId, View};
+use tui::layout::Rect;
 
 use std::path::PathBuf;
-use std::{cell::RefCell, rc::Rc};
 
 use slotmap::{DefaultKey as Key, SlotMap};
 
@@ -9,7 +9,7 @@ use anyhow::Error;
 
 pub struct Editor {
     pub tree: Tree,
-    pub documents: SlotMap<DocumentId, Rc<RefCell<Document>>>,
+    pub documents: SlotMap<DocumentId, Document>,
     pub count: Option<usize>,
     pub theme: Theme,
     pub language_servers: helix_lsp::Registry,
@@ -34,18 +34,25 @@ impl Editor {
         }
     }
 
-    pub fn open(&mut self, path: PathBuf) -> Result<(), Error> {
-        // TODO: issues with doc already being borrowed if called from inside goto()
+    fn _refresh(&mut self) {
+        for (view, _) in self.tree.views_mut() {
+            let doc = &self.documents[view.doc];
+            view.ensure_cursor_in_view(doc)
+        }
+    }
 
-        let existing_view = self
-            .tree
-            .views()
-            .find(|(view, _)| view.doc.borrow().path() == Some(&path));
+    pub fn open(&mut self, path: PathBuf) -> Result<DocumentId, Error> {
+        let existing_view = self.documents().find(|doc| doc.path() == Some(&path));
 
-        if let Some((view, _)) = existing_view {
-            self.tree.focus = view.id;
-            return Ok(());
-        }
+        // TODO:
+        // if view with doc, focus it
+        // else open new split
+
+        // if let Some((view, _)) = existing_view {
+        //     let id = view.doc.id;
+        //     self.tree.focus = view.id;
+        //     return Ok(id);
+        // }
 
         let mut doc = Document::load(path, self.theme.scopes())?;
 
@@ -73,13 +80,14 @@ impl Editor {
             .unwrap();
         }
 
-        let doc = Rc::new(RefCell::new(doc));
-        // TODO: store id as doc.id
-        let id = self.documents.insert(doc.clone());
+        let id = self.documents.insert(doc);
+        self.documents[id].id = id;
 
-        let view = View::new(doc)?;
+        let view = View::new(id)?;
         self.tree.insert(view);
-        Ok(())
+        self._refresh();
+
+        Ok(id)
     }
 
     pub fn close(&mut self, id: Key) {
@@ -88,7 +96,7 @@ impl Editor {
         let language_servers = &mut self.language_servers;
         let executor = self.executor;
 
-        let doc = view.doc.borrow();
+        let doc = &self.documents[view.doc];
 
         let language_server = doc
             .language
@@ -99,14 +107,14 @@ impl Editor {
             smol::block_on(language_server.text_document_did_close(doc.identifier())).unwrap();
         }
 
-        drop(doc); // to stop borrowing self.tree
-
         // self.documents.remove(view.doc);
         self.tree.remove(id);
+        self._refresh();
     }
 
-    pub fn resize(&mut self) {
-        self.tree.focus_next();
+    pub fn resize(&mut self, area: Rect) {
+        self.tree.resize(area);
+        self._refresh();
     }
 
     pub fn focus_next(&mut self) {
@@ -125,12 +133,26 @@ impl Editor {
         self.tree.get_mut(self.tree.focus)
     }
 
+    pub fn ensure_cursor_in_view(&mut self, id: Key) {
+        let view = self.tree.get_mut(id);
+        let doc = &self.documents[view.doc];
+        view.ensure_cursor_in_view(doc)
+    }
+
+    pub fn document(&self, id: DocumentId) -> Option<&Document> {
+        self.documents.get(id)
+    }
+
+    pub fn documents(&self) -> impl Iterator<Item = &Document> {
+        self.documents.iter().map(|(_id, doc)| doc)
+    }
+
     pub fn cursor_position(&self) -> Option<helix_core::Position> {
         const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter
         let view = self.view();
-        let doc = view.doc.borrow();
+        let doc = &self.documents[view.doc];
         let cursor = doc.selection().cursor();
-        if let Some(mut pos) = view.screen_coords_at_pos(doc.text().slice(..), cursor) {
+        if let Some(mut pos) = view.screen_coords_at_pos(doc, doc.text().slice(..), cursor) {
             pos.col += view.area.x as usize + OFFSET as usize;
             pos.row += view.area.y as usize;
             return Some(pos);
diff --git a/helix-view/src/tree.rs b/helix-view/src/tree.rs
index 0a487b6f..e56566e3 100644
--- a/helix-view/src/tree.rs
+++ b/helix-view/src/tree.rs
@@ -164,7 +164,18 @@ impl Tree {
         self.recalculate()
     }
 
-    pub fn views(&mut self) -> impl Iterator<Item = (&mut View, bool)> {
+    pub fn views(&self) -> impl Iterator<Item = (&View, bool)> {
+        let focus = self.focus;
+        self.nodes.iter().filter_map(move |(key, node)| match node {
+            Node {
+                content: Content::View(view),
+                ..
+            } => Some((view.as_ref(), focus == key)),
+            _ => None,
+        })
+    }
+
+    pub fn views_mut(&mut self) -> impl Iterator<Item = (&mut View, bool)> {
         let focus = self.focus;
         self.nodes
             .iter_mut()
@@ -231,7 +242,6 @@ impl Tree {
                 Content::View(view) => {
                     // debug!!("setting view area {:?}", area);
                     view.area = area;
-                    view.ensure_cursor_in_view();
                 } // TODO: call f()
                 Content::Container(container) => {
                     // debug!!("setting container area {:?}", area);
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 2a77f2c0..9d3f1412 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -1,7 +1,6 @@
 use anyhow::Error;
 
 use std::borrow::Cow;
-use std::{cell::RefCell, rc::Rc};
 
 use crate::{Document, DocumentId};
 use helix_core::{
@@ -15,13 +14,13 @@ pub const PADDING: usize = 5;
 
 pub struct View {
     pub id: Key,
-    pub doc: Rc<RefCell<Document>>,
+    pub doc: DocumentId,
     pub first_line: usize,
     pub area: Rect,
 }
 
 impl View {
-    pub fn new(doc: Rc<RefCell<Document>>) -> Result<Self, Error> {
+    pub fn new(doc: DocumentId) -> Result<Self, Error> {
         let view = Self {
             id: Key::default(),
             doc,
@@ -32,8 +31,7 @@ impl View {
         Ok(view)
     }
 
-    pub fn ensure_cursor_in_view(&mut self) {
-        let doc = self.doc.borrow();
+    pub fn ensure_cursor_in_view(&mut self, doc: &Document) {
         let cursor = doc.selection().cursor();
         let line = doc.text().char_to_line(cursor);
         let document_end = self.first_line + (self.area.height as usize).saturating_sub(2);
@@ -51,8 +49,7 @@ impl View {
 
     /// Calculates the last visible line on screen
     #[inline]
-    pub fn last_line(&self) -> usize {
-        let doc = self.doc.borrow();
+    pub fn last_line(&self, doc: &Document) -> usize {
         let height = self.area.height.saturating_sub(1); // - 1 for statusline
         std::cmp::min(
             self.first_line + height as usize,
@@ -63,10 +60,15 @@ impl View {
     /// Translates a document position to an absolute position in the terminal.
     /// Returns a (line, col) position if the position is visible on screen.
     // TODO: Could return width as well for the character width at cursor.
-    pub fn screen_coords_at_pos(&self, text: RopeSlice, pos: usize) -> Option<Position> {
+    pub fn screen_coords_at_pos(
+        &self,
+        doc: &Document,
+        text: RopeSlice,
+        pos: usize,
+    ) -> Option<Position> {
         let line = text.char_to_line(pos);
 
-        if line < self.first_line || line > self.last_line() {
+        if line < self.first_line || line > self.last_line(doc) {
             // Line is not visible on screen
             return None;
         }
@@ -74,7 +76,7 @@ impl View {
         let line_start = text.line_to_char(line);
         let line_slice = text.slice(line_start..pos);
         let mut col = 0;
-        let tab_width = self.doc.borrow().tab_width();
+        let tab_width = doc.tab_width();
 
         for grapheme in RopeGraphemes::new(line_slice) {
             if grapheme == "\t" {