From 798dbd27c50296d260fe13483e4a3aacd240c6ad Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Mon, 22 Mar 2021 12:21:33 +0900 Subject: 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 2e7104cd..86c8c03f 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -126,6 +126,7 @@ impl Range { } /// 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 @@ impl Selection { // 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 @@ impl<'a> IntoIterator for &'a Selection { } } -// 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 1f9e63aa..f25ee208 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -464,11 +464,13 @@ impl Transaction { I: IntoIterator + 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" -- cgit v1.2.3-70-g09d2