Merge pull request #211 from atom/ns-prepopulated-folds

Handle out-of-range folds when populating the spatial index

spec/display-layer-spec.js

@@ -454,6 +454,80 @@ describe('DisplayLayer', () => {
       expect(displayLayer.getText()).toBe('a⋯i jkzvwx')
       expect(displayLayer.foldsIntersectingBufferRange([[0, 0], [Infinity, 0]]).length).toBe(1)
     })
+
+    it('accounts for pre-populated folds that end/start on the same row when populating an empty index', () => {
+      const buffer = new TextBuffer({
+        text: 'abc\ndef\nghi\njkl\nmno\npqr'
+      })
+      const foldsMarkerLayer = buffer.addMarkerLayer()
+      foldsMarkerLayer.markRange([[1, 2], [2, 1]])
+      foldsMarkerLayer.markRange([[2, 2], [3, 1]])
+      foldsMarkerLayer.markRange([[3, 2], [4, 1]])
+      foldsMarkerLayer.markRange([[4, 2], [5, 1]])
+      const displayLayer = buffer.addDisplayLayer({foldsMarkerLayer})
+
+      expect(displayLayer.indexedBufferRowCount).toBe(0)
+      displayLayer.foldBufferRange([[0, 2], [1, 1]])
+      expect(displayLayer.getText()).toBe('ab⋯e⋯h⋯k⋯n⋯qr')
+    })
+
+    it('accounts for pre-populated folds with intersecting ranges when populating an empty index', () => {
+      const buffer = new TextBuffer({
+        text: 'abc\ndef\nghi\njkl\nmno\npqr'
+      })
+      const foldsMarkerLayer = buffer.addMarkerLayer()
+      foldsMarkerLayer.markRange([[1, 2], [2, 2]])
+      foldsMarkerLayer.markRange([[2, 1], [3, 2]])
+      foldsMarkerLayer.markRange([[3, 1], [4, 2]])
+      foldsMarkerLayer.markRange([[4, 2], [5, 1]])
+      const displayLayer = buffer.addDisplayLayer({foldsMarkerLayer})
+
+      expect(displayLayer.indexedBufferRowCount).toBe(0)
+      displayLayer.foldBufferRange([[0, 2], [1, 1]])
+      expect(displayLayer.getText()).toBe('ab⋯e⋯⋯qr')
+    })
+
+    it('accounts for random pre-populated folds when populating an empty index', () => {
+      const now = Date.now()
+
+      for (let i = 0; i < 100; i++) {
+        let seed = now + i
+
+        try {
+          const random = new Random(seed)
+          const buffer = new TextBuffer({
+            text: buildRandomLines(random, 40)
+          })
+          const foldsMarkerLayer = buffer.addMarkerLayer({
+            maintainHistory: false,
+            persistent: true,
+            destroyInvalidatedMarkers: true
+          })
+          for (let i = 0, n = random(20); i < n; i++) {
+            foldsMarkerLayer.markRange(getRandomBufferRange(random, buffer))
+          }
+
+          const displayLayer = buffer.addDisplayLayer({foldsMarkerLayer})
+
+          const randomRange = getRandomBufferRange(random, buffer)
+
+          // In displayLayerCopy, our reference, we'll create a fold after fully populating the spatial index
+          const displayLayerCopy = displayLayer.copy()
+          displayLayerCopy.getText() // force a full index
+          expect(displayLayerCopy.indexedBufferRowCount).toBe(buffer.getLineCount())
+          displayLayerCopy.foldBufferRange(randomRange)
+
+          // In displayLayer, we'll create a fold before poulating the spatial index.
+          expect(displayLayer.indexedBufferRowCount).toBe(0)
+          displayLayer.foldBufferRange(randomRange)
+
+          expect(displayLayer.getText()).toBe(displayLayerCopy.getText())
+        } catch (error) {
+          console.log(`Failing Seed: ${seed}`)
+          throw error
+        }
+      }
+    })
   })
 
   describe('soft wraps', () => {

src/display-layer.js

@@ -198,7 +198,9 @@ class DisplayLayer {
   foldBufferRange (bufferRange) {
     bufferRange = Range.fromObject(bufferRange)
     const containingFoldMarkers = this.foldsMarkerLayer.findMarkers({containsRange: bufferRange})
-    this.populateSpatialIndexIfNeeded(bufferRange.end.row + 1, Infinity)
+    if (containingFoldMarkers.length === 0) {
+      this.populateSpatialIndexIfNeeded(bufferRange.end.row + 1, Infinity)
+    }
     const foldId = this.foldsMarkerLayer.markRange(bufferRange, {invalidate: 'overlap', exclusive: true}).id
     if (containingFoldMarkers.length === 0) {
       const foldStartRow = bufferRange.start.row
@@ -1049,6 +1051,25 @@ class DisplayLayer {
       intersectsRowRange: [startBufferRow, endBufferRow - 1]
     })
 
+    // If the given buffer range exceeds the indexed range, we need to ensure
+    // we consider any folds that intersect the combined row range of the
+    // initially-queried folds, since we couldn't use the index to expand the
+    // row range to account for these extra folds ahead of time.
+    if (endBufferRow >= this.indexedBufferRowCount) {
+      for (let i = 0; i < foldMarkers.length; i++) {
+        const marker = foldMarkers[i]
+        const nextMarker = foldMarkers[i + 1]
+        if (marker.getEndPosition().row >= endBufferRow &&
+            (!nextMarker || nextMarker.getEndPosition().row < marker.getEndPosition().row)) {
+          const intersectingMarkers = this.foldsMarkerLayer.findMarkers({
+            intersectsRow: marker.getEndPosition().row
+          })
+          endBufferRow = marker.getEndPosition().row + 1
+          foldMarkers.splice(i, foldMarkers.length - i, ...intersectingMarkers)
+        }
+      }
+    }
+
     for (let i = 0; i < foldMarkers.length; i++) {
       const foldStart = foldMarkers[i].getStartPosition()
       let foldEnd = foldMarkers[i].getEndPosition()