Skip to content

Commit

Permalink
fix: moving span end past the start
Browse files Browse the repository at this point in the history
This bug occurs if:
* A span needs to be subliced at the end of an in-progress span
* Another shorter span exists at the same poisitiotion
* That span ends before the in-progress span does

An exammple would look roughly like this:

     D
   |--|
       C
   |-------|
        B
   |----------|
     A
|---------|

Because spans at the same position are sorted in the descending order,
the largest span (B) is processed first and subslicing  is started.
The current code then moves the start of all subslices at the
same position to the end of the of the in-progress span (A).
This is correctt in many cases (like C) where the spans at the same poistion
extend past the end of the in-progress span (A).
However if any spans (C) are shorted than the in-progress span (A),
the start of these slices will be moved past their end.

To fix this, the code now first splits all spans that end before
the end of the in-progress span (A) (in this case B and C) into subslices.
This is identical to the previous behaviour except that the subslicing
stops as soon as the first span whose end starts before the end of the
in-progress span (A) is encountered.

Afterwards the span list is resorted (unchanged) to ensure the new
subslices are at the correct posistion.

Finally the spans that are shorter than the in-progress span (A)
are processed normally (no subslicing).
  • Loading branch information
pascalkuthe committed Sep 25, 2022
1 parent 276e58b commit f73a14e
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 39 deletions.
80 changes: 41 additions & 39 deletions helix-core/src/syntax/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,49 +139,33 @@ impl Iterator for SpanIter {

self.cursor = span.start;

// Handle all spans that share this starting point. Either subslice
// or fully consume the span.
let mut i = self.index;
let mut subslices = 0;
loop {
match self.spans.get_mut(i) {
Some(span) if span.start == self.cursor => {
self.event_queue
.push_back(HighlightStart(Highlight(span.scope)));
i += 1;

match subslice {
Some(intersect) => {
// If this span needs to be subsliced, consume the
// left part of the subslice and leave the right.
self.range_ends.push(intersect);
span.start = intersect;
subslices += 1;
}
None => {
// If there is no subslice, consume the span.
self.range_ends.push(span.end);
self.index = i;
}
}
if let Some(intersect) = subslice {
// Handle all spans that share this starting point. Either subslice
// or fully consume the span.
let mut i = self.index;
// If this span needs to be subsliced, consume the
// left part of the subslice and leave the right.
while let Some(span) = self.spans.get_mut(i) {
if span.start != self.cursor || span.end < intersect {
break;
}
_ => break,

self.event_queue
.push_back(HighlightStart(Highlight(span.scope)));
self.range_ends.push(intersect);
span.start = intersect;
i += 1;
}
}

// Ensure range-ends are sorted ascending. Ranges which start at the
// same point may be in descending order because of the assumed
// sort-order of input ranges.
self.range_ends.sort_unstable();
let subslices = i - self.index;

// When spans are subsliced, the span Vec may need to be re-sorted
// because the `range.start` may now be greater than some `range.start`
// later in the Vec. This is not a classic "sort": we take several
// shortcuts to improve the runtime so that the sort may be done in
// time linear to the cardinality of the span Vec. Practically speaking
// the runtime is even better since we only scan from `self.index` to
// the first element of the Vec with a `range.start` after this range.
if let Some(intersect) = subslice {
// When spans are subsliced, the span Vec may need to be re-sorted
// because the `range.start` may now be greater than some `range.start`
// later in the Vec. This is not a classic "sort": we take several
// shortcuts to improve the runtime so that the sort may be done in
// time linear to the cardinality of the span Vec. Practically speaking
// the runtime is even better since we only scan from `self.index` to
// the first element of the Vec with a `range.start` after this range.
let mut after = None;

// Find the index of the largest span smaller than the intersect point.
Expand All @@ -206,6 +190,24 @@ impl Iterator for SpanIter {
}
}

for span in &self.spans[self.index..] {
if span.start != self.cursor {
break;
}

self.event_queue
.push_back(HighlightStart(Highlight(span.scope)));
self.range_ends.push(span.end);

// If there is no subslice, consume the span.
self.index += 1;
}

// Ensure range-ends are sorted ascending. Ranges which start at the
// same point may be in descending order because of the assumed
// sort-order of input ranges.
self.range_ends.sort_unstable();

self.event_queue.pop_front()
}
}
Expand Down
97 changes: 97 additions & 0 deletions helix-core/src/syntax/span/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,100 @@ fn empty_span_at_sublice_start() {
],
);
}

#[test]
fn intercept_in_subslice() {
use HighlightEvent::*;
/*
Input:
3
|-------|
2
|----------------|
1
|-----------|
|---|---|---|---|---|
0 1 2 3 4 5 */
let input = vec![span!(1, 0..3), span!(2, 1..5), span!(3, 2..4)];

/*
Output:
3
|---|
2 3
|-------|---|
1 2
|-----------|-------|
|---|---|---|---|---|
0 1 2 3 4 5 */
let output: Vec<_> = span_iter(input).collect();
assert_eq!(
output,
&[
HighlightStart(Highlight(1)),
Source { start: 0, end: 1 },
HighlightStart(Highlight(2)),
Source { start: 1, end: 2 },
HighlightStart(Highlight(3)),
Source { start: 2, end: 3 },
HighlightEnd, // ends 3
HighlightEnd, // ends 2
HighlightEnd, // ends 1
HighlightStart(Highlight(2)),
HighlightStart(Highlight(3)),
Source { start: 3, end: 4 },
HighlightEnd, // ends 3
Source { start: 4, end: 5 },
HighlightEnd, // ends 2
],
);
}

#[test]
fn subslice_in_intercept() {
use HighlightEvent::*;
/*
Input:
3
|---|
2
|-----------|
1
|-----------|
|---|---|---|---|---|
0 1 2 3 4 5 */
let input = vec![span!(1, 0..3), span!(2, 1..4), span!(3, 1..2)];

/*
Output:
3
|---|
2
|-------|
1 2
|-----------|---|
|---|---|---|---|
0 1 2 3 4 */
let output: Vec<_> = span_iter(input).collect();
assert_eq!(
output,
&[
HighlightStart(Highlight(1)),
Source { start: 0, end: 1 },
HighlightStart(Highlight(2)),
HighlightStart(Highlight(3)),
Source { start: 1, end: 2 },
HighlightEnd, // ends 3
Source { start: 2, end: 3 },
HighlightEnd, // ends 2
HighlightEnd, // ends 1
HighlightStart(Highlight(2)),
Source { start: 3, end: 4 },
HighlightEnd, // ends 2
],
);
}

0 comments on commit f73a14e

Please sign in to comment.