From 798dbd27c50296d260fe13483e4a3aacd240c6ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= <blaz@mxxn.io>
Date: Mon, 22 Mar 2021 12:21:33 +0900
Subject: [PATCH] Selection: fail early if new() is called with no ranges.

---
 helix-core/src/selection.rs   | 13 +++++++++++--
 helix-core/src/transaction.rs |  4 +++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs
index 2e7104cd7..86c8c03f2 100644
--- a/helix-core/src/selection.rs
+++ b/helix-core/src/selection.rs
@@ -126,6 +126,7 @@ pub fn fragment<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> Cow<'b, str> {
 }
 
 /// A selection consists of one or more selection ranges.
+/// invariant: A selection can never be empty (always contains at least primary range).
 #[derive(Debug, Clone)]
 pub struct Selection {
     ranges: SmallVec<[Range; 1]>,
@@ -205,12 +206,14 @@ pub fn point(pos: usize) -> Self {
     // TODO: consume an iterator or a vec to reduce allocations?
     #[must_use]
     pub fn new(ranges: SmallVec<[Range; 1]>, primary_index: usize) -> Self {
+        assert!(!ranges.is_empty());
+
         fn normalize(mut ranges: SmallVec<[Range; 1]>, mut primary_index: usize) -> Selection {
             let primary = ranges[primary_index];
             ranges.sort_unstable_by_key(Range::from);
             primary_index = ranges.iter().position(|&range| range == primary).unwrap();
 
-            let mut result: SmallVec<[Range; 1]> = SmallVec::new();
+            let mut result = SmallVec::new();
 
             // TODO: we could do with one vec by removing elements as we mutate
 
@@ -294,7 +297,7 @@ fn into_iter(self) -> std::slice::Iter<'a, Range> {
     }
 }
 
-// TODO: checkSelection -> check if valid for doc length
+// TODO: checkSelection -> check if valid for doc length && sorted
 
 pub fn keep_matches(
     text: RopeSlice,
@@ -387,6 +390,12 @@ pub fn split_on_matches(
 mod test {
     use super::*;
 
+    #[test]
+    #[should_panic]
+    fn test_new_empty() {
+        let sel = Selection::new(smallvec![], 0);
+    }
+
     #[test]
     fn test_create_normalizes_and_merges() {
         let sel = Selection::new(
diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs
index 1f9e63aa6..f25ee2082 100644
--- a/helix-core/src/transaction.rs
+++ b/helix-core/src/transaction.rs
@@ -464,11 +464,13 @@ pub fn change<I>(doc: &Rope, changes: I) -> Self
         I: IntoIterator<Item = Change> + ExactSizeIterator,
     {
         let len = doc.len_chars();
-        let acc = Vec::with_capacity(2 * changes.len() + 1);
+        let acc = Vec::with_capacity(2 * changes.len() + 1); // rough estimate
         let mut changeset = ChangeSet { changes: acc, len };
 
         // TODO: verify ranges are ordered and not overlapping or change will panic.
 
+        // TODO: test for (pos, pos, None) to factor out as nothing
+
         let mut last = 0;
         for (from, to, tendril) in changes {
             // Retain from last "to" to current "from"