From bf7d7e0d2ad714e121741d503d5801660b284f61 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Fri, 18 Sep 2015 10:17:53 +0200 Subject: [PATCH] Improve LinesYardstick design We have shifted the responsibility of orchestrating state updates and measurements to the yardstick. The presenter still needs to be updated to make use of these new capabilities. --- spec/lines-yardstick-spec.coffee | 39 ++++++++----------- ...der.coffee => mock-lines-component.coffee} | 14 ++++--- spec/text-editor-presenter-spec.coffee | 2 +- src/lines-yardstick.coffee | 8 +++- 4 files changed, 31 insertions(+), 32 deletions(-) rename spec/{mock-line-nodes-provider.coffee => mock-lines-component.coffee} (85%) diff --git a/spec/lines-yardstick-spec.coffee b/spec/lines-yardstick-spec.coffee index 23842e6ba..e68ac15e5 100644 --- a/spec/lines-yardstick-spec.coffee +++ b/spec/lines-yardstick-spec.coffee @@ -1,8 +1,8 @@ LinesYardstick = require '../src/lines-yardstick' -MockLineNodesProvider = require './mock-line-nodes-provider' +MockLinesComponent = require './mock-lines-component' describe "LinesYardstick", -> - [editor, mockLineNodesProvider, builtLineNodes, linesYardstick] = [] + [editor, mockPresenter, mockLinesComponent, linesYardstick] = [] beforeEach -> waitsForPromise -> @@ -12,14 +12,23 @@ describe "LinesYardstick", -> atom.project.open('sample.js').then (o) -> editor = o runs -> - mockLineNodesProvider = new MockLineNodesProvider(editor) - linesYardstick = new LinesYardstick(editor, mockLineNodesProvider) + mockPresenter = {getStateForMeasurements: jasmine.createSpy()} + mockLinesComponent = new MockLinesComponent(editor) + linesYardstick = new LinesYardstick(editor, mockPresenter, mockLinesComponent) + + mockLinesComponent.setDefaultFont("14px monospace") afterEach -> - mockLineNodesProvider.dispose() + doSomething = true it "converts screen positions to pixel positions", -> - mockLineNodesProvider.setDefaultFont("14px monospace") + 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}] @@ -37,7 +46,7 @@ describe "LinesYardstick", -> linesYardstick.pixelPositionForScreenPosition(point) ).toEqual(position) - mockLineNodesProvider.setFontForScopes( + mockLinesComponent.setFontForScopes( ["source.js", "storage.modifier.js"], "16px monospace" ) linesYardstick.clearCache() @@ -57,19 +66,3 @@ describe "LinesYardstick", -> expect( linesYardstick.pixelPositionForScreenPosition(point) ).toEqual(position) - - it "does not compute the same position twice unless the cache gets cleared", -> - mockLineNodesProvider.setDefaultFont("14px monospace") - - oldPosition1 = linesYardstick.pixelPositionForScreenPosition([0, 5]) - oldPosition2 = linesYardstick.pixelPositionForScreenPosition([1, 4]) - - mockLineNodesProvider.setDefaultFont("16px monospace") - - expect(linesYardstick.pixelPositionForScreenPosition([0, 5])).toEqual(oldPosition1) - expect(linesYardstick.pixelPositionForScreenPosition([1, 4])).toEqual(oldPosition2) - - linesYardstick.clearCache() - - expect(linesYardstick.pixelPositionForScreenPosition([0, 5])).not.toEqual(oldPosition1) - expect(linesYardstick.pixelPositionForScreenPosition([1, 4])).not.toEqual(oldPosition2) diff --git a/spec/mock-line-nodes-provider.coffee b/spec/mock-lines-component.coffee similarity index 85% rename from spec/mock-line-nodes-provider.coffee rename to spec/mock-lines-component.coffee index c0944a771..fd8d04cbc 100644 --- a/spec/mock-line-nodes-provider.coffee +++ b/spec/mock-lines-component.coffee @@ -1,8 +1,8 @@ TokenIterator = require '../src/token-iterator' module.exports = -class MockLineNodesProvider - constructor: (@editor) -> +class MockLinesComponent + constructor: (@model) -> @defaultFont = "" @fontsByScopes = {} @tokenIterator = new TokenIterator @@ -11,14 +11,12 @@ class MockLineNodesProvider dispose: -> node.remove() for node in @builtLineNodes - setFontForScopes: (scopes, font) -> @fontsByScopes[scopes] = font - - setDefaultFont: (font) -> @defaultFont = font + updateSync: jasmine.createSpy() lineNodeForLineIdAndScreenRow: (id, screenRow) -> lineNode = document.createElement("div") lineNode.style.whiteSpace = "pre" - lineState = @editor.tokenizedLineForScreenRow(screenRow) + lineState = @model.tokenizedLineForScreenRow(screenRow) @tokenIterator.reset(lineState) while @tokenIterator.next() @@ -32,3 +30,7 @@ class MockLineNodesProvider document.body.appendChild(lineNode) lineNode + + setFontForScopes: (scopes, font) -> @fontsByScopes[scopes] = font + + setDefaultFont: (font) -> @defaultFont = font diff --git a/spec/text-editor-presenter-spec.coffee b/spec/text-editor-presenter-spec.coffee index 4f662294e..60de8293b 100644 --- a/spec/text-editor-presenter-spec.coffee +++ b/spec/text-editor-presenter-spec.coffee @@ -5,7 +5,7 @@ TextBuffer = require 'text-buffer' TextEditor = require '../src/text-editor' TextEditorPresenter = require '../src/text-editor-presenter' LinesYardstick = require '../src/lines-yardstick' -MockLineNodesProvider = require './mock-line-nodes-provider' +MockLineNodesProvider = require './mock-lines-component' describe "TextEditorPresenter", -> # These `describe` and `it` blocks mirror the structure of the ::state object. diff --git a/src/lines-yardstick.coffee b/src/lines-yardstick.coffee index 0e86e24ca..ab1177071 100644 --- a/src/lines-yardstick.coffee +++ b/src/lines-yardstick.coffee @@ -4,14 +4,18 @@ AcceptFilter = {acceptNode: -> NodeFilter.FILTER_ACCEPT} module.exports = class LinesYardstick - constructor: (@model, @lineNodesProvider) -> + constructor: (@model, @presenter, @lineNodesProvider) -> + @cachedPositionsByLineId = {} @tokenIterator = new TokenIterator @rangeForMeasurement = document.createRange() - @cachedPositionsByLineId = {} clearCache: -> @cachedPositionsByLineId = {} + prepareScreenRowsForMeasurement: (screenRows) -> + state = @presenter.getStateForMeasurements(screenRows) + @lineNodesProvider.updateSync(state) + pixelPositionForScreenPosition: (screenPosition, clip=true) -> screenPosition = Point.fromObject(screenPosition) screenPosition = @model.clipScreenPosition(screenPosition) if clip