on master help sign in
Change the `Range` type and associated functions to gap indexing.
commitc1b0a7197556ec0383d6b0d0e7f510ecf4dcd21f authorNathan Vegdahl <cessen@cessen.com> dateFri 25 Jun 2021, 01:40 (5 years ago) parent79f0969
1 file changed+126−56
helix-core/src/selection.rs+126−56
helix-core/src/selection.rs+126−56
@@ -1,5 +1,5 @@
1−//! Selections are the primary editing construct. Even a single cursor is defined as an empty
2−//! single selection range.
1+//! Selections are the primary editing construct. Even a single cursor is
2+//! defined as a single empty or 1-wide selection range.
33//!
44//! All positioning is done via `char` offsets into the buffer.
55use crate::{Assoc, ChangeSet, Rope, RopeSlice};
@@ -15,16 +15,39 @@ fn abs_difference(x: usize, y: usize) -> usize {
1515 }
1616}
1717
18−/// A single selection range. Anchor-inclusive, head-exclusive.
18+/// A single selection range.
19+///
20+/// The range consists of an "anchor" and "head" position in
21+/// the text. The head is the part that the user moves when
22+/// directly extending the selection. The head and anchor
23+/// can be in any order: either can precede or follow the
24+/// other in the text, and they can share the same position
25+/// for a zero-width range.
26+///
27+/// Below are some example `Range` configurations to better
28+/// illustrate. The anchor and head indices are show as
29+/// "(anchor, head)", followed by example text with "[" and "]"
30+/// inserted to visually represent the anchor and head positions:
31+///
32+/// - (0, 3): [Som]e text.
33+/// - (3, 0): ]Som[e text.
34+/// - (2, 7): So[me te]xt.
35+/// - (1, 1): S[]ome text.
36+///
37+/// Ranges are considered to be inclusive on the left and
38+/// exclusive on the right, regardless of anchor-head ordering.
39+/// This means, for example, that non-zero-width ranges that
40+/// are directly adjecent, sharing an edge, do not overlap.
41+/// However, a zero-width range will overlap with the shared
42+/// left-edge of another range.
1943#[derive(Debug, Clone, Copy, PartialEq, Eq)]
2044pub struct Range {
21− // TODO: optimize into u32
2245 /// The anchor of the range: the side that doesn't move when extending.
2346 pub anchor: usize,
2447 /// The head of the range, moved when extending.
2548 pub head: usize,
2649 pub horiz: Option<u32>,
27−} // TODO: might be cheaper to store normalized as from/to and an inverted flag
50+}
2851
2952impl Range {
3053 pub fn new(anchor: usize, head: usize) -> Self {
@@ -62,25 +85,14 @@ impl Range {
6285 /// Check two ranges for overlap.
6386 #[must_use]
6487 pub fn overlaps(&self, other: &Self) -> bool {
65− // cursor overlap is checked differently
66− if self.is_empty() {
67− let pos = self.head;
68− pos >= other.from() && other.to() >= pos
69− } else {
70− self.to() > other.from() && other.to() > self.from()
71− }
88+ // To my eye, it's non-obvious why this works, but I arrived
89+ // at it after transforming the slower version that explicitly
90+ // enumerated more cases. The unit tests are thorough.
91+ self.from() == other.from() || (self.to() > other.from() && other.to() > self.from())
7292 }
7393
7494 pub fn contains(&self, pos: usize) -> bool {
75− if self.is_empty() {
76− return false;
77− }
78−
79− if self.anchor < self.head {
80− self.anchor <= pos && pos < self.head
81− } else {
82− self.head < pos && pos <= self.anchor
83− }
95+ self.from() <= pos && pos < self.to()
8496 }
8597
8698 /// Map a range through a set of changes. Returns a new range representing the same position
@@ -89,10 +101,10 @@ impl Range {
89101 let anchor = changes.map_pos(self.anchor, Assoc::After);
90102 let head = changes.map_pos(self.head, Assoc::After);
91103
92− // TODO: possibly unnecessary
93− if self.anchor == anchor && self.head == head {
94− return self;
95− }
104+ // We want to return a new `Range` with `horiz == None` every time,
105+ // even if the anchor and head haven't changed, because we don't
106+ // know if the *visual* position hasn't changed due to
107+ // character-width or grapheme changes earlier in the text.
96108 Self {
97109 anchor,
98110 head,
@@ -103,22 +115,20 @@ impl Range {
103115 /// Extend the range to cover at least `from` `to`.
104116 #[must_use]
105117 pub fn extend(&self, from: usize, to: usize) -> Self {
106− if from <= self.anchor && to >= self.anchor {
107− return Self {
108− anchor: from,
109− head: to,
110− horiz: None,
111− };
112− }
118+ debug_assert!(from <= to);
113119
114− Self {
115− anchor: self.anchor,
116− head: if abs_difference(from, self.anchor) > abs_difference(to, self.anchor) {
117− from
118− } else {
119− to
120− },
121− horiz: None,
120+ if self.anchor <= self.head {
121+ Self {
122+ anchor: self.anchor.min(from),
123+ head: self.head.max(to),
124+ horiz: None,
125+ }
126+ } else {
127+ Self {
128+ anchor: self.anchor.max(to),
129+ head: self.head.min(from),
130+ horiz: None,
131+ }
122132 }
123133 }
124134
@@ -126,7 +136,7 @@ impl Range {
126136
127137 #[inline]
128138 pub fn fragment<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> Cow<'b, str> {
129− Cow::from(text.slice(self.from()..self.to() + 1))
139+ text.slice(self.from()..self.to()).into()
130140 }
131141}
132142
@@ -355,7 +365,7 @@ pub fn select_on_matches(
355365
356366 let start = text.byte_to_char(start_byte + mat.start());
357367 let end = text.byte_to_char(start_byte + mat.end());
358− result.push(Range::new(start, end.saturating_sub(1)));
368+ result.push(Range::new(start, end));
359369 }
360370 }
361371
@@ -376,6 +386,12 @@ pub fn split_on_matches(
376386 let mut result = SmallVec::with_capacity(selection.len());
377387
378388 for sel in selection {
389+ // Special case: zero-width selection.
390+ if sel.from() == sel.to() {
391+ result.push(*sel);
392+ continue;
393+ }
394+
379395 // TODO: can't avoid occasional allocations since Regex can't operate on chunks yet
380396 let fragment = sel.fragment(text);
381397
@@ -388,13 +404,12 @@ pub fn split_on_matches(
388404
389405 for mat in regex.find_iter(&fragment) {
390406 // TODO: retain range direction
391−
392407 let end = text.byte_to_char(start_byte + mat.start());
393− result.push(Range::new(start, end.saturating_sub(1)));
408+ result.push(Range::new(start, end));
394409 start = text.byte_to_char(start_byte + mat.end());
395410 }
396411
397− if start <= sel_end {
412+ if start < sel_end {
398413 result.push(Range::new(start, sel_end));
399414 }
400415 }
@@ -475,7 +490,7 @@ mod test {
475490 .collect::<Vec<String>>()
476491 .join(",");
477492
478− assert_eq!(res, "8/10,10/12");
493+ assert_eq!(res, "8/10,10/12,12/12");
479494 }
480495
481496 #[test]
@@ -489,35 +504,90 @@ mod test {
489504 assert_eq!(range.contains(13), false);
490505
491506 let range = Range::new(9, 6);
492− assert_eq!(range.contains(9), true);
507+ assert_eq!(range.contains(9), false);
493508 assert_eq!(range.contains(7), true);
494− assert_eq!(range.contains(6), false);
509+ assert_eq!(range.contains(6), true);
510+ }
511+
512+ #[test]
513+ fn test_overlaps() {
514+ // Two non-zero-width ranges, no overlap.
515+ assert!(!Range::new(0, 3).overlaps(&Range::new(3, 6)));
516+ assert!(!Range::new(0, 3).overlaps(&Range::new(6, 3)));
517+ assert!(!Range::new(3, 0).overlaps(&Range::new(3, 6)));
518+ assert!(!Range::new(3, 0).overlaps(&Range::new(6, 3)));
519+ assert!(!Range::new(3, 6).overlaps(&Range::new(0, 3)));
520+ assert!(!Range::new(3, 6).overlaps(&Range::new(3, 0)));
521+ assert!(!Range::new(6, 3).overlaps(&Range::new(0, 3)));
522+ assert!(!Range::new(6, 3).overlaps(&Range::new(3, 0)));
523+
524+ // Two non-zero-width ranges, overlap.
525+ assert!(Range::new(0, 4).overlaps(&Range::new(3, 6)));
526+ assert!(Range::new(0, 4).overlaps(&Range::new(6, 3)));
527+ assert!(Range::new(4, 0).overlaps(&Range::new(3, 6)));
528+ assert!(Range::new(4, 0).overlaps(&Range::new(6, 3)));
529+ assert!(Range::new(3, 6).overlaps(&Range::new(0, 4)));
530+ assert!(Range::new(3, 6).overlaps(&Range::new(4, 0)));
531+ assert!(Range::new(6, 3).overlaps(&Range::new(0, 4)));
532+ assert!(Range::new(6, 3).overlaps(&Range::new(4, 0)));
533+
534+ // Zero-width and non-zero-width range, no overlap.
535+ assert!(!Range::new(0, 3).overlaps(&Range::new(3, 3)));
536+ assert!(!Range::new(3, 0).overlaps(&Range::new(3, 3)));
537+ assert!(!Range::new(3, 3).overlaps(&Range::new(0, 3)));
538+ assert!(!Range::new(3, 3).overlaps(&Range::new(3, 0)));
539+
540+ // Zero-width and non-zero-width range, overlap.
541+ assert!(Range::new(1, 4).overlaps(&Range::new(1, 1)));
542+ assert!(Range::new(4, 1).overlaps(&Range::new(1, 1)));
543+ assert!(Range::new(1, 1).overlaps(&Range::new(1, 4)));
544+ assert!(Range::new(1, 1).overlaps(&Range::new(4, 1)));
545+
546+ assert!(Range::new(1, 4).overlaps(&Range::new(3, 3)));
547+ assert!(Range::new(4, 1).overlaps(&Range::new(3, 3)));
548+ assert!(Range::new(3, 3).overlaps(&Range::new(1, 4)));
549+ assert!(Range::new(3, 3).overlaps(&Range::new(4, 1)));
550+
551+ // Two zero-width ranges, no overlap.
552+ assert!(!Range::new(0, 0).overlaps(&Range::new(1, 1)));
553+ assert!(!Range::new(1, 1).overlaps(&Range::new(0, 0)));
554+
555+ // Two zero-width ranges, overlap.
556+ assert!(Range::new(1, 1).overlaps(&Range::new(1, 1)));
495557 }
496558
497559 #[test]
498560 fn test_split_on_matches() {
499561 use crate::regex::Regex;
500562
501− let text = Rope::from("abcd efg wrs xyz 123 456");
563+ let text = Rope::from(" abcd efg wrs xyz 123 456");
502564
503− let selection = Selection::new(smallvec![Range::new(0, 8), Range::new(10, 19),], 0);
565+ let selection = Selection::new(smallvec![Range::new(0, 9), Range::new(11, 20),], 0);
504566
505567 let result = split_on_matches(text.slice(..), &selection, &Regex::new(r"\s+").unwrap());
506568
507569 assert_eq!(
508570 result.ranges(),
509571 &[
510− Range::new(0, 3),
511− Range::new(5, 7),
512− Range::new(10, 11),
513− Range::new(15, 17),
514− Range::new(19, 19),
572+ // We get a leading zero-width range when there's
573+ // a leading match because ranges are inclusive on
574+ // the left. Imagine, for example, if the entire
575+ // selection range were matched: you'd still want
576+ // at least one range to remain after the split.
577+ Range::new(0, 0),
578+ Range::new(1, 5),
579+ Range::new(6, 9),
580+ Range::new(11, 13),
581+ Range::new(16, 19),
582+ // In contrast to the comment above, there is no
583+ // _trailing_ zero-width range despite the trailing
584+ // match, because ranges are exclusive on the right.
515585 ]
516586 );
517587
518588 assert_eq!(
519589 result.fragments(text.slice(..)).collect::<Vec<_>>(),
520− &["abcd", "efg", "rs", "xyz", "1"]
590+ &["", "abcd", "efg", "rs", "xyz"]
521591 );
522592 }
523593}