Fix initial selection of Document in new view

When a new View of a Document is created, a default cursor of 0, 0 is
created, and it does not get normalized to a single width cursor until
at least one movement of the cursor happens. This appears to have no
practical negative effect that I could find, but it makes tests difficult
to work with, since the initial selection is not what you expect it to be.

This changes the initial selection of a new View to be the width of the
first grapheme in the text.

.gitignore

@@ -1,6 +1,5 @@
 target
 .direnv
 helix-term/rustfmt.toml
-helix-syntax/languages/
 result
 runtime/grammars

helix-core/src/auto_pairs.rs

@@ -1,9 +1,7 @@
 //! When typing the opening character of one of the possible pairs defined below,
 //! this module provides the functionality to insert the paired closing character.
 
-use crate::{
+use crate::{graphemes, movement::Direction, Range, Rope, Selection, Tendril, Transaction};
-    graphemes, movement::Direction, Range, Rope, RopeGraphemes, Selection, Tendril, Transaction,
-};
 use std::collections::HashMap;
 
 use log::debug;
@@ -149,14 +147,6 @@ fn prev_char(doc: &Rope, pos: usize) -> Option<char> {
     doc.get_char(pos - 1)
 }
 
-fn is_single_grapheme(doc: &Rope, range: &Range) -> bool {
-    let mut graphemes = RopeGraphemes::new(doc.slice(range.from()..range.to()));
-    let first = graphemes.next();
-    let second = graphemes.next();
-    debug!("first: {:#?}, second: {:#?}", first, second);
-    first.is_some() && second.is_none()
-}
-
 /// calculate what the resulting range should be for an auto pair insertion
 fn get_next_range(
     doc: &Rope,
@@ -189,8 +179,8 @@ fn get_next_range(
         );
     }
 
-    let single_grapheme = is_single_grapheme(doc, start_range);
     let doc_slice = doc.slice(..);
+    let single_grapheme = start_range.is_single_grapheme(doc_slice);
 
     // just skip over graphemes
     if len_inserted == 0 {

helix-core/src/selection.rs

@@ -8,7 +8,7 @@ use crate::{
         prev_grapheme_boundary,
     },
     movement::Direction,
-    Assoc, ChangeSet, RopeSlice,
+    Assoc, ChangeSet, RopeGraphemes, RopeSlice,
 };
 use smallvec::{smallvec, SmallVec};
 use std::borrow::Cow;
@@ -339,6 +339,14 @@ impl Range {
     pub fn cursor_line(&self, text: RopeSlice) -> usize {
         text.char_to_line(self.cursor(text))
     }
+
+    /// Returns true if this Range covers a single grapheme in the given text
+    pub fn is_single_grapheme(&self, doc: RopeSlice) -> bool {
+        let mut graphemes = RopeGraphemes::new(doc.slice(self.from()..self.to()));
+        let first = graphemes.next();
+        let second = graphemes.next();
+        first.is_some() && second.is_none()
+    }
 }
 
 impl From<(usize, usize)> for Range {

helix-loader/Cargo.toml

@@ -20,7 +20,6 @@ toml = "0.5"
 etcetera = "0.4"
 tree-sitter = "0.20"
 once_cell = "1.12"
-
 log = "0.4"
 
 # TODO: these two should be on !wasm32 only

helix-loader/src/lib.rs

@@ -13,7 +13,9 @@ pub fn runtime_dir() -> std::path::PathBuf {
 
     if let Ok(dir) = std::env::var("CARGO_MANIFEST_DIR") {
         // this is the directory of the crate being run by cargo, we need the workspace path so we take the parent
-        return std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR);
+        let path = std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR);
+        log::debug!("runtime dir: {}", path.to_string_lossy());
+        return path;
     }
 
     const RT_DIR: &str = "runtime";

helix-term/src/application.rs

@@ -55,8 +55,29 @@ pub struct Application {
     lsp_progress: LspProgressMap,
 }
 
+#[cfg(feature = "integration")]
+fn setup_integration_logging() {
+    // Separate file config so we can include year, month and day in file logs
+    let _ = fern::Dispatch::new()
+        .format(|out, message, record| {
+            out.finish(format_args!(
+                "{} {} [{}] {}",
+                chrono::Local::now().format("%Y-%m-%dT%H:%M:%S%.3f"),
+                record.target(),
+                record.level(),
+                message
+            ))
+        })
+        .level(log::LevelFilter::Debug)
+        .chain(std::io::stdout())
+        .apply();
+}
+
 impl Application {
     pub fn new(args: Args, config: Config) -> Result<Self, Error> {
+        #[cfg(feature = "integration")]
+        setup_integration_logging();
+
         use helix_view::editor::Action;
 
         let config_dir = helix_loader::config_dir();

helix-term/src/commands.rs

@@ -2094,10 +2094,17 @@ fn insert_mode(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
     enter_insert_mode(doc);
 
-    let selection = doc
+    log::trace!(
-        .selection(view.id)
+        "entering insert mode with sel: {:?}, text: {:?}",
-        .clone()
+        doc.selection(view.id),
-        .transform(|range| Range::new(range.to(), range.from()));
+        doc.text().to_string()
+    );
+
+    let selection = doc.selection(view.id).clone().transform(|range| {
+        let new_range = Range::new(range.to(), range.from());
+        new_range
+    });
+
     doc.set_selection(view.id, selection);
 }
 
@@ -2444,8 +2451,8 @@ fn normal_mode(cx: &mut Context) {
                 graphemes::prev_grapheme_boundary(text, range.to()),
             )
         });
-        doc.set_selection(view.id, selection);
 
+        doc.set_selection(view.id, selection);
         doc.restore_cursor = false;
     }
 }

helix-term/tests/integration.rs

@@ -2,9 +2,9 @@
 mod integration {
     use std::path::PathBuf;
 
-    use helix_core::{syntax::AutoPairConfig, Position, Selection, Tendril, Transaction};
+    use helix_core::{syntax::AutoPairConfig, Position, Selection, Transaction};
     use helix_term::{application::Application, args::Args, config::Config};
-    use helix_view::{current, doc, input::parse_macro};
+    use helix_view::{doc, input::parse_macro};
 
     use crossterm::event::{Event, KeyEvent};
     use indoc::indoc;
@@ -25,14 +25,14 @@ mod integration {
         let mut app =
             app.unwrap_or_else(|| Application::new(Args::default(), Config::default()).unwrap());
 
-        let (view, doc) = current!(app.editor);
+        let (view, doc) = helix_view::current!(app.editor);
+        let sel = doc.selection(view.id).clone();
 
+        // replace the initial text with the input text
         doc.apply(
-            &Transaction::insert(
+            &Transaction::change_by_selection(&doc.text(), &sel, |_| {
-                doc.text(),
+                (0, doc.text().len_chars(), Some((&test_case.in_text).into()))
-                &Selection::single(1, 0),
+            })
-                Tendril::from(&test_case.in_text),
-            )
             .with_selection(test_case.in_selection.clone()),
             view.id,
         );
@@ -80,12 +80,70 @@ mod integration {
             Args::default(),
             Config::default(),
             TestCase {
-                in_text: String::new(),
+                in_text: "\n".into(),
                 in_selection: Selection::single(0, 1),
                 // TODO: fix incorrect selection on new doc
-                in_keys: String::from("ihello world<esc>hl"),
+                in_keys: "ihello world<esc>".into(),
-                out_text: String::from("hello world\n"),
+                out_text: "hello world\n".into(),
-                out_selection: Selection::single(11, 12),
+                out_selection: Selection::single(12, 11),
+            },
+        )?;
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn insert_mode_cursor_position() -> anyhow::Result<()> {
+        test_key_sequence_text_result(
+            Args::default(),
+            Config::default(),
+            TestCase {
+                in_text: String::new(),
+                in_selection: Selection::single(0, 0),
+                in_keys: "i".into(),
+                out_text: String::new(),
+                out_selection: Selection::single(0, 0),
+            },
+        )?;
+
+        test_key_sequence_text_result(
+            Args::default(),
+            Config::default(),
+            TestCase {
+                in_text: "\n".into(),
+                in_selection: Selection::single(0, 1),
+                in_keys: "i".into(),
+                out_text: "\n".into(),
+                out_selection: Selection::single(1, 0),
+            },
+        )?;
+
+        test_key_sequence_text_result(
+            Args::default(),
+            Config::default(),
+            TestCase {
+                in_text: "\n".into(),
+                in_selection: Selection::single(0, 1),
+                in_keys: "i<esc>i".into(),
+                out_text: "\n".into(),
+                out_selection: Selection::single(1, 0),
+            },
+        )?;
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> {
+        test_key_sequence_text_result(
+            Args::default(),
+            Config::default(),
+            TestCase {
+                in_text: "\n".into(),
+                in_selection: Selection::single(0, 1),
+                in_keys: "i".into(),
+                out_text: "\n".into(),
+                out_selection: Selection::single(1, 0),
             },
         )?;
 
@@ -98,11 +156,11 @@ mod integration {
             Args::default(),
             Config::default(),
             TestCase {
-                in_text: String::new(),
+                in_text: "\n".into(),
                 in_selection: Selection::single(0, 1),
-                in_keys: String::from("i(<esc>hl"),
+                in_keys: "i(<esc>".into(),
-                out_text: String::from("()\n"),
+                out_text: "()\n".into(),
-                out_selection: Selection::single(1, 2),
+                out_selection: Selection::single(2, 1),
             },
         )?;
 
@@ -116,11 +174,11 @@ mod integration {
                 ..Default::default()
             },
             TestCase {
-                in_text: String::new(),
+                in_text: "\n".into(),
                 in_selection: Selection::single(0, 1),
-                in_keys: String::from("i(<esc>hl"),
+                in_keys: "i(<esc>".into(),
-                out_text: String::from("(\n"),
+                out_text: "(\n".into(),
-                out_selection: Selection::single(1, 2),
+                out_selection: Selection::single(2, 1),
             },
         )?;
 
@@ -136,15 +194,17 @@ mod integration {
             },
             Config::default(),
             TestCase {
-                in_text: String::from("void foo() {}"),
+                in_text: "void foo() {}\n".into(),
-                in_selection: Selection::single(12, 13),
+                in_selection: Selection::single(13, 12),
-                in_keys: String::from("i<ret><esc>"),
+                in_keys: "i<ret><esc>".into(),
-                out_text: String::from(indoc! {r#"
+                out_text: indoc! {r#"
                     void foo() {
                       
                     }
-                "#}),
+                "#}
-                out_selection: Selection::single(15, 16),
+                .trim_start()
+                .into(),
+                out_selection: Selection::single(16, 15),
             },
         )?;
 

helix-view/src/document.rs

@@ -1,5 +1,6 @@
 use anyhow::{anyhow, bail, Context, Error};
 use helix_core::auto_pairs::AutoPairs;
+use helix_core::Range;
 use serde::de::{self, Deserialize, Deserializer};
 use serde::Serialize;
 use std::cell::Cell;
@@ -83,7 +84,7 @@ impl Serialize for Mode {
 pub struct Document {
     pub(crate) id: DocumentId,
     text: Rope,
-    pub(crate) selections: HashMap<ViewId, Selection>,
+    selections: HashMap<ViewId, Selection>,
 
     path: Option<PathBuf>,
     encoding: &'static encoding::Encoding,
@@ -637,6 +638,37 @@ impl Document {
             .insert(view_id, selection.ensure_invariants(self.text().slice(..)));
     }
 
+    /// Find the origin selection of the text in a document, i.e. where
+    /// a single cursor would go if it were on the first grapheme. If
+    /// the text is empty, returns (0, 0).
+    pub fn origin(&self) -> Range {
+        if self.text().len_chars() == 0 {
+            return Range::new(0, 0);
+        }
+
+        Range::new(0, 1).grapheme_aligned(self.text().slice(..))
+    }
+
+    /// Reset the view's selection on this document to the
+    /// [origin](Document::origin) cursor.
+    pub fn reset_selection(&mut self, view_id: ViewId) {
+        let origin = self.origin();
+        self.set_selection(view_id, Selection::single(origin.anchor, origin.head));
+    }
+
+    /// Initializes a new selection for the given view if it does not
+    /// already have one.
+    pub fn ensure_view_init(&mut self, view_id: ViewId) {
+        if self.selections.get(&view_id).is_none() {
+            self.reset_selection(view_id);
+        }
+    }
+
+    /// Remove a view's selection from this document.
+    pub fn remove_view(&mut self, view_id: ViewId) {
+        self.selections.remove(&view_id);
+    }
+
     /// Apply a [`Transaction`] to the [`Document`] to change its text.
     fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {
         let old_doc = self.text().clone();

helix-view/src/editor.rs

@@ -32,12 +32,12 @@ use anyhow::{bail, Error};
 
 pub use helix_core::diagnostic::Severity;
 pub use helix_core::register::Registers;
+use helix_core::Position;
 use helix_core::{
     auto_pairs::AutoPairs,
     syntax::{self, AutoPairConfig},
     Change,
 };
-use helix_core::{Position, Selection};
 use helix_dap as dap;
 
 use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer};
@@ -645,11 +645,8 @@ impl Editor {
         view.offset = Position::default();
 
         let doc = self.documents.get_mut(&doc_id).unwrap();
+        doc.ensure_view_init(view.id);
 
-        // initialize selection for view
-        doc.selections
-            .entry(view.id)
-            .or_insert_with(|| Selection::point(0));
         // TODO: reuse align_view
         let pos = doc
             .selection(view.id)
@@ -719,9 +716,7 @@ impl Editor {
             Action::Load => {
                 let view_id = view!(self).id;
                 let doc = self.documents.get_mut(&id).unwrap();
-                if doc.selections().is_empty() {
+                doc.ensure_view_init(view_id);
-                    doc.set_selection(view_id, Selection::point(0));
-                }
                 return;
             }
             Action::HorizontalSplit | Action::VerticalSplit => {
@@ -736,7 +731,7 @@ impl Editor {
                 );
                 // initialize selection for view
                 let doc = self.documents.get_mut(&id).unwrap();
-                doc.set_selection(view_id, Selection::point(0));
+                doc.ensure_view_init(view_id);
             }
         }
 
@@ -769,7 +764,7 @@ impl Editor {
         Ok(self.new_file_from_document(action, Document::from(rope, Some(encoding))))
     }
 
-    // ???
+    // ??? possible use for integration tests
     pub fn open(&mut self, path: PathBuf, action: Action) -> Result<DocumentId, Error> {
         let path = helix_core::path::get_canonicalized_path(&path)?;
         let id = self.document_by_path(&path).map(|doc| doc.id);
@@ -791,12 +786,7 @@ impl Editor {
     pub fn close(&mut self, id: ViewId) {
         let view = self.tree.get(self.tree.focus);
         // remove selection
-        self.documents
+        self.documents.get_mut(&view.doc).unwrap().remove_view(id);
-            .get_mut(&view.doc)
-            .unwrap()
-            .selections
-            .remove(&id);
-
         self.tree.remove(id);
         self._refresh();
     }
@@ -871,7 +861,7 @@ impl Editor {
             let view = View::new(doc_id, self.config().gutters.clone());
             let view_id = self.tree.insert(view);
             let doc = self.documents.get_mut(&doc_id).unwrap();
-            doc.set_selection(view_id, Selection::point(0));
+            doc.ensure_view_init(view_id);
         }
 
         self._refresh();