Redesign LinesYardstick

spec/lines-yardstick-spec.coffee

@@ -1,68 +0,0 @@
-LinesYardstick = require '../src/lines-yardstick'
-MockLinesComponent = require './mock-lines-component'
-
-describe "LinesYardstick", ->
-  [editor, mockPresenter, mockLinesComponent, linesYardstick] = []
-
-  beforeEach ->
-    waitsForPromise ->
-      atom.packages.activatePackage('language-javascript')
-
-    waitsForPromise ->
-      atom.project.open('sample.js').then (o) -> editor = o
-
-    runs ->
-      mockPresenter = {getStateForMeasurements: jasmine.createSpy()}
-      mockLinesComponent = new MockLinesComponent(editor)
-      linesYardstick = new LinesYardstick(editor, mockPresenter, mockLinesComponent)
-
-      mockLinesComponent.setDefaultFont("14px monospace")
-
-  afterEach ->
-    doSomething = true
-
-  it "converts screen positions to pixel positions", ->
-    stubState = {anything: {}}
-    mockPresenter.getStateForMeasurements.andReturn(stubState)
-
-    linesYardstick.prepareScreenRowsForMeasurement([0, 1, 2])
-
-    expect(mockPresenter.getStateForMeasurements).toHaveBeenCalledWith([0, 1, 2])
-    expect(mockLinesComponent.updateSync).toHaveBeenCalledWith(stubState)
-
-    conversionTable = [
-      [[0, 0], {left: 0, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 3], {left: 24, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 4], {left: 32, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 5], {left: 40, top: editor.getLineHeightInPixels() * 0}]
-      [[1, 0], {left: 0, top: editor.getLineHeightInPixels() * 1}]
-      [[1, 1], {left: 0, top: editor.getLineHeightInPixels() * 1}]
-      [[1, 6], {left: 48, top: editor.getLineHeightInPixels() * 1}]
-      [[1, Infinity], {left: 240, top: editor.getLineHeightInPixels() * 1}]
-    ]
-
-    for [point, position] in conversionTable
-      expect(
-        linesYardstick.pixelPositionForScreenPosition(point)
-      ).toEqual(position)
-
-    mockLinesComponent.setFontForScopes(
-      ["source.js", "storage.modifier.js"], "16px monospace"
-    )
-    linesYardstick.clearCache()
-
-    conversionTable = [
-      [[0, 0], {left: 0, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 3], {left: 30, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 4], {left: 38, top: editor.getLineHeightInPixels() * 0}]
-      [[0, 5], {left: 46, top: editor.getLineHeightInPixels() * 0}]
-      [[1, 0], {left: 0, top: editor.getLineHeightInPixels() * 1}]
-      [[1, 1], {left: 0, top: editor.getLineHeightInPixels() * 1}]
-      [[1, 6], {left: 54, top: editor.getLineHeightInPixels() * 1}]
-      [[1, Infinity], {left: 246, top: editor.getLineHeightInPixels() * 1}]
-    ]
-
-    for [point, position] in conversionTable
-      expect(
-        linesYardstick.pixelPositionForScreenPosition(point)
-      ).toEqual(position)

spec/text-editor-presenter-spec.coffee

@@ -4,15 +4,17 @@ TextBuffer = require 'text-buffer'
 {Point, Range} = TextBuffer
 TextEditor = require '../src/text-editor'
 TextEditorPresenter = require '../src/text-editor-presenter'
+LinesYardstick = require '../src/lines-yardstick'
 
-fdescribe "TextEditorPresenter", ->
-  [buffer, editor] = []
+describe "TextEditorPresenter", ->
+  [buffer, editor, linesYardstick, mockLineNodesProvider] = []
 
   beforeEach ->
     # These *should* be mocked in the spec helper, but changing that now would break packages :-(
     spyOn(window, "setInterval").andCallFake window.fakeSetInterval
     spyOn(window, "clearInterval").andCallFake window.fakeClearInterval
 
+    mockLineNodesProvider = {updateSync: ->}
     buffer = new TextBuffer(filePath: require.resolve('./fixtures/sample.js'))
     editor = new TextEditor({buffer})
     waitsForPromise -> buffer.load()
@@ -37,7 +39,10 @@ fdescribe "TextEditorPresenter", ->
       scrollTop: 0
       scrollLeft: 0
 
-    new TextEditorPresenter(params)
+    presenter = new TextEditorPresenter(params)
+    linesYardstick = new LinesYardstick(editor, presenter, mockLineNodesProvider)
+    presenter.setLinesYardstick(linesYardstick)
+    presenter
 
   expectValues = (actual, expected) ->
     for key, value of expected
@@ -55,19 +60,6 @@ fdescribe "TextEditorPresenter", ->
 
   expectNoStateUpdate = (presenter, fn) -> expectStateUpdatedToBe(false, presenter, fn)
 
-  describe "::getStateForMeasurements(screenRows)", ->
-    it "contains states for tiles of the visible rows + the supplied ones", ->
-      presenter = buildPresenter(explicitHeight: 6, scrollTop: 0, lineHeight: 1, tileSize: 2)
-      state = presenter.getStateForMeasurements([10, 11])
-
-      expect(state.content.tiles[0]).toBeDefined()
-      expect(state.content.tiles[2]).toBeDefined()
-      expect(state.content.tiles[4]).toBeDefined()
-      expect(state.content.tiles[6]).toBeDefined()
-      expect(state.content.tiles[8]).toBeUndefined()
-      expect(state.content.tiles[10]).toBeDefined()
-      expect(state.content.tiles[12]).toBeUndefined()
-
   # These `describe` and `it` blocks mirror the structure of the ::state object.
   # Please maintain this structure when adding specs for new state fields.
   describe "::getState()", ->

src/lines-component.coffee

@@ -97,3 +97,7 @@ class LinesComponent extends TiledComponent
       component.clearMeasurements()
 
     @presenter.clearScopedCharacterWidths()
+
+  lineNodeForLineIdAndScreenRow: (lineId, screenRow) ->
+    tile = @presenter.tileForRow(screenRow)
+    @getComponentForTile(screenRow)?.lineNodeForLineId(lineId)

src/lines-tile-component.coffee

@@ -399,3 +399,5 @@ class LinesTileComponent
 
   clearMeasurements: ->
     @measuredLines.clear()
+
+  lineNodeForLineId: (id) -> @lineNodesByLineId[id]

src/lines-yardstick.coffee

@@ -13,8 +13,17 @@ class LinesYardstick
     @cachedPositionsByLineId = {}
 
   prepareScreenRowsForMeasurement: (screenRows) ->
-    state = @presenter.getStateForMeasurements(screenRows)
-    @lineNodesProvider.updateSync(state)
+    @presenter.setScreenRowsToMeasure(screenRows)
+    @lineNodesProvider.updateSync(@presenter.getStateForMeasurements())
+
+  cleanup: ->
+    @presenter.clearScreenRowsToMeasure()
+    @lineNodesProvider.updateSync(@presenter.getStateForMeasurements())
+
+  measure: (screenRows, fn) ->
+    @prepareScreenRowsForMeasurement(screenRows)
+    fn()
+    @cleanup()
 
   pixelPositionForScreenPosition: (screenPosition, clip=true) ->
     screenPosition = Point.fromObject(screenPosition)
@@ -47,7 +56,7 @@ class LinesYardstick
 
     @tokenIterator.reset(tokenizedLine)
     while @tokenIterator.next()
-      break if indexWithinTextNode?
+      break if foundIndexWithinTextNode?
 
       text = @tokenIterator.getText()
 
@@ -75,16 +84,16 @@ class LinesYardstick
           nextTextNodeIndex = textNodeIndex + textNodeLength
 
         if charIndex is column
-          indexWithinTextNode = charIndex - textNodeIndex
+          foundIndexWithinTextNode = charIndex - textNodeIndex
           break
 
         charIndex += charLength
 
     if textNode?
-      indexWithinTextNode ?= textNode.textContent.length
+      foundIndexWithinTextNode ?= textNode.textContent.length
       @cachedPositionsByLineId[tokenizedLine.id] ?= {}
       @cachedPositionsByLineId[tokenizedLine.id][column] =
-        @leftPixelPositionForCharInTextNode(lineNode, textNode, indexWithinTextNode)
+        @leftPixelPositionForCharInTextNode(lineNode, textNode, foundIndexWithinTextNode)
     else
       0
 

src/text-editor-component.coffee

@@ -13,6 +13,7 @@ ScrollbarComponent = require './scrollbar-component'
 ScrollbarCornerComponent = require './scrollbar-corner-component'
 OverlayManager = require './overlay-manager'
 DOMElementPool = require './dom-element-pool'
+LinesYardstick = require './lines-yardstick'
 
 module.exports =
 class TextEditorComponent
@@ -79,6 +80,9 @@ class TextEditorComponent
     @linesComponent = new LinesComponent({@presenter, @hostElement, @useShadowDOM, @domElementPool})
     @scrollViewNode.appendChild(@linesComponent.getDomNode())
 
+    @linesYardstick = new LinesYardstick(@editor, @presenter, @linesComponent)
+    @presenter.setLinesYardstick(@linesYardstick)
+
     @horizontalScrollbarComponent = new ScrollbarComponent({orientation: 'horizontal', onScroll: @onHorizontalScroll})
     @scrollViewNode.appendChild(@horizontalScrollbarComponent.getDomNode())
 

src/text-editor-presenter.coffee

@@ -67,16 +67,17 @@ class TextEditorPresenter
   isBatching: ->
     @updating is false
 
-  getStateForMeasurements: (screenRows) ->
+  getStateForMeasurements: ->
     @updateVerticalDimensions()
     @updateScrollbarDimensions()
     @updateStartRow()
     @updateEndRow()
 
-    screenRows = _.without(screenRows, null, undefined, @getVisibleRows()...)
+    @updateLineDecorations() if @shouldUpdateDecorations
+    @updateTilesState() if @shouldUpdateLineNumbersState or @shouldUpdateLinesState
 
-    @updateVisibleTilesState() if @shouldUpdateLineNumbersState or @shouldUpdateLinesState
-    @updateTilesState(screenRows, true)
+    @shouldUpdateLinesState = false
+    @shouldUpdateLineNumbersState = false
 
     @state
 
@@ -85,29 +86,25 @@ class TextEditorPresenter
   getState: ->
     @updating = true
 
-    @linesYardstick.prepareScreenRowsForMeasurement([
-      @model.getLongestScreenRow()
-    ])
-    @deleteTemporaryTiles()
+    @linesYardstick.measure [@model.getLongestScreenRow()], =>
+      @updateCommonGutterState()
+      @updateHorizontalDimensions()
+      @updateReflowState()
 
-    @updateCommonGutterState()
-    @updateHorizontalDimensions()
-    @updateReflowState()
+      @updateFocusedState() if @shouldUpdateFocusedState
+      @updateHeightState() if @shouldUpdateHeightState
+      @updateVerticalScrollState() if @shouldUpdateVerticalScrollState
+      @updateHorizontalScrollState() if @shouldUpdateHorizontalScrollState
+      @updateScrollbarsState() if @shouldUpdateScrollbarsState
+      @updateHiddenInputState() if @shouldUpdateHiddenInputState
+      @updateContentState() if @shouldUpdateContentState
+      @updateHighlightDecorations() if @shouldUpdateDecorations
+      @updateCursorsState() if @shouldUpdateCursorsState
+      @updateOverlaysState() if @shouldUpdateOverlaysState
+      @updateLineNumberGutterState() if @shouldUpdateLineNumberGutterState
+      @updateGutterOrderState() if @shouldUpdateGutterOrderState
+      @updateCustomGutterDecorationState() if @shouldUpdateCustomGutterDecorationState
 
-    @updateFocusedState() if @shouldUpdateFocusedState
-    @updateHeightState() if @shouldUpdateHeightState
-    @updateVerticalScrollState() if @shouldUpdateVerticalScrollState
-    @updateHorizontalScrollState() if @shouldUpdateHorizontalScrollState
-    @updateScrollbarsState() if @shouldUpdateScrollbarsState
-    @updateHiddenInputState() if @shouldUpdateHiddenInputState
-    @updateContentState() if @shouldUpdateContentState
-    @updateDecorations() if @shouldUpdateDecorations
-    @updateVisibleTilesState() if @shouldUpdateLinesState or @shouldUpdateLineNumbersState
-    @updateCursorsState() if @shouldUpdateCursorsState
-    @updateOverlaysState() if @shouldUpdateOverlaysState
-    @updateLineNumberGutterState() if @shouldUpdateLineNumberGutterState
-    @updateGutterOrderState() if @shouldUpdateGutterOrderState
-    @updateCustomGutterDecorationState() if @shouldUpdateCustomGutterDecorationState
     @updating = false
 
     @resetTrackedUpdates()
@@ -354,25 +351,31 @@ class TextEditorPresenter
       @tileForRow(@model.getScreenLineCount()), @tileForRow(@endRow)
     )
 
-  getVisibleRows: ->
+  getVisibleScreenRows: ->
     startRow = @getStartTileRow()
     endRow = Math.min(@model.getScreenLineCount(), @getEndTileRow() + @tileSize)
 
     [startRow...endRow]
 
-  updateVisibleTilesState: ->
-    @updateTilesState(@getVisibleRows())
+  getScreenRows: ->
+    screenRows = @getVisibleScreenRows().concat(@screenRowsToMeasure)
+    screenRows.sort (a, b) -> a - b
+    _.uniq(screenRows, true)
 
-  deleteTemporaryTiles: ->
-    for tileId, tileState of @state.content.tiles when tileState.temporary
-      delete @state.content.tiles[tileId]
-      delete @lineNumberGutter.tiles[tileId]
+  setScreenRowsToMeasure: (screenRows) ->
+    @screenRowsToMeasure = screenRows
+    @shouldUpdateLinesState = true
+    @shouldUpdateDecorations = true
 
-  updateTilesState: (screenRows, temporary = false) ->
+  clearScreenRowsToMeasure: ->
+    @screenRowsToMeasure = []
+    @shouldUpdateLinesState = true
+    @shouldUpdateDecorations = true
+
+  updateTilesState: ->
     return unless @startRow? and @endRow? and @lineHeight?
 
-    screenRows.sort (a, b) -> a - b
-
+    screenRows = @getScreenRows()
     visibleTiles = {}
     startRow = screenRows[0]
     endRow = screenRows[screenRows.length - 1]
@@ -398,14 +401,12 @@ class TextEditorPresenter
       tile.display = "block"
       tile.zIndex = zIndex
       tile.highlights ?= {}
-      tile.temporary = temporary
 
       gutterTile = @lineNumberGutter.tiles[tileStartRow] ?= {}
       gutterTile.top = tileStartRow * @lineHeight - @scrollTop
       gutterTile.height = @tileSize * @lineHeight
       gutterTile.display = "block"
       gutterTile.zIndex = zIndex
-      gutterTile.temporary = temporary
 
       @updateLinesState(tile, rowsWithinTile) if @shouldUpdateLinesState
       @updateLineNumbersState(gutterTile, rowsWithinTile) if @shouldUpdateLineNumbersState
@@ -413,8 +414,6 @@ class TextEditorPresenter
       visibleTiles[tileStartRow] = true
       zIndex++
 
-    return if temporary
-
     if @mouseWheelScreenRow? and @model.tokenizedLineForScreenRow(@mouseWheelScreenRow)?
       mouseWheelTile = @tileForRow(@mouseWheelScreenRow)
 
@@ -1194,22 +1193,30 @@ class TextEditorPresenter
 
     @emitDidUpdateState()
 
-  updateDecorations: ->
+  updateLineDecorations: ->
     @rangesByDecorationId = {}
     @lineDecorationsByScreenRow = {}
     @lineNumberDecorationsByScreenRow = {}
     @customGutterDecorationsByGutterNameAndScreenRow = {}
+
+    return unless 0 <= @startRow <= @endRow <= Infinity
+
+    for row in @getScreenRows()
+      for markerId, decorations of @model.decorationsForScreenRowRange(row, row)
+        range = @model.getMarker(markerId).getScreenRange()
+        for decoration in decorations
+          if decoration.isType('line') or decoration.isType('gutter')
+            @addToLineDecorationCaches(decoration, range)
+
+  updateHighlightDecorations: ->
     @visibleHighlights = {}
 
     return unless 0 <= @startRow <= @endRow <= Infinity
 
     for markerId, decorations of @model.decorationsForScreenRowRange(@startRow, @endRow - 1)
       range = @model.getMarker(markerId).getScreenRange()
-      for decoration in decorations
-        if decoration.isType('line') or decoration.isType('gutter')
-          @addToLineDecorationCaches(decoration, range)
-        else if decoration.isType('highlight')
-          @updateHighlightState(decoration, range)
+      for decoration in decorations when decoration.isType('highlight')
+        @updateHighlightState(decoration, range)
 
     for tileId, tileState of @state.content.tiles
       for id, highlight of tileState.highlights