Parameterize style/className retrieval in MarkerTextDecorationLayer

With the previous design it started to feel like we were adding
unnecessary indirection to add a marker text decoration layer. With this
commit we are taking a step back: `addMarkerTextDecorationLayer` got
deleted and the `MarkerTextDecorationLayer` constructor was exposed
instead.

This allows to pass parameters to `MarkerTextDecorationLayer`, which
will stop being in charge of storing and retrieving class names and
inline styles associated to a scope id, leaving that decision up to the
caller.

spec/display-layer-spec.js

@@ -1,6 +1,7 @@
 const Random = require('random-seed')
 const dedent = require('dedent')
 const TextBuffer = require('../src/text-buffer')
+const MarkerTextDecorationLayer = require('../src/marker-text-decoration-layer')
 const Point = require('../src/point')
 const Range = require('../src/range')
 const {buildRandomLines, getRandomBufferRange} = require('./helpers/random')
@@ -1796,12 +1797,21 @@ describe('DisplayLayer', () => {
       const buffer = new TextBuffer({text: 'abcde\nfghij\nklmno'})
       const displayLayer = buffer.addDisplayLayer()
       const markerLayer = buffer.addMarkerLayer()
-      const textDecorationLayer = displayLayer.addMarkerTextDecorationLayer(markerLayer)
+
+      const classNamesByMarkerId = new Map()
+      const inlineStylesByMarkerId = new Map()
+      displayLayer.addTextDecorationLayer(new MarkerTextDecorationLayer(
+        markerLayer,
+        {
+          classNameForMarkerId: (id) => classNamesByMarkerId.get(id),
+          inlineStyleForMarkerId: (id) => inlineStylesByMarkerId.get(id)
+        }
+      ))
       const marker1 = markerLayer.markRange([[0, 0], [0, 1]])
-      textDecorationLayer.setClassNameForMarker(marker1, 'a')
+      classNamesByMarkerId.set(marker1.id, 'a')
+      inlineStylesByMarkerId.set(marker1.id, {color: 'b'})
       const marker2 = markerLayer.markRange([[1, 0], [1, 3]])
-      textDecorationLayer.setInlineStyleForMarker(marker1, {color: 'b'})
-      textDecorationLayer.setInlineStyleForMarker(marker2, {color: 'c'})
+      inlineStylesByMarkerId.set(marker2.id, {color: 'c'})
 
       const [screenLine1, screenLine2] = displayLayer.getScreenLines(0, 2)
       expect(screenLine1.tags.length).toBe(4)
@@ -1813,10 +1823,10 @@ describe('DisplayLayer', () => {
       expect(screenLine1.tags[3]).toBe(4)
 
       expect(screenLine2.tags.length).toBe(4)
-      expect(displayLayer.classNameForTag(screenLine2.tags[0])).toBeNull()
+      expect(displayLayer.classNameForTag(screenLine2.tags[0])).toBeUndefined()
       expect(displayLayer.inlineStyleForTag(screenLine2.tags[0])).toEqual({color: 'c'})
       expect(screenLine2.tags[1]).toBe(3)
-      expect(displayLayer.classNameForTag(screenLine2.tags[2])).toBeNull()
+      expect(displayLayer.classNameForTag(screenLine2.tags[2])).toBeUndefined()
       expect(displayLayer.inlineStyleForTag(screenLine2.tags[2])).toEqual({color: 'c'})
       expect(screenLine2.tags[3]).toBe(2)
     })
@@ -2266,29 +2276,15 @@ describe('DisplayLayer', () => {
 
       try {
         const random = new Random(seed)
-
-        const buffer = new TextBuffer({
-          text: buildRandomLines(random, 20)
-        })
-
-        const invisibles = {}
-
-        if (random(2) > 0) {
-          invisibles.space = '•'
-        }
-
-        if (random(2) > 0) {
-          invisibles.eol = EOL_INVISIBLE
-        }
-
-        if (random(2) > 0) {
-          invisibles.cr = CR_INVISIBLE
-        }
-
+        const buffer = new TextBuffer({text: buildRandomLines(random, 20)})
         const foldIds = []
         const showIndentGuides = Boolean(random(2))
         const softWrapColumn = random(2) ? random.intBetween(5, 80) : null
         const foldsMarkerLayer = random(2) ? createFoldsMarkerLayer(random, buffer, foldIds) : null
+        const invisibles = {}
+        if (random(2) > 0) invisibles.space = '•'
+        if (random(2) > 0) invisibles.eol = EOL_INVISIBLE
+        if (random(2) > 0) invisibles.cr = CR_INVISIBLE
 
         const displayLayer = buffer.addDisplayLayer({
           tabLength: 4,
@@ -2298,9 +2294,10 @@ describe('DisplayLayer', () => {
           foldsMarkerLayer: foldsMarkerLayer
         })
 
+        const classNamesByMarkerId = new Map()
         const decorationLayersCount = random(4)
         for (let i = 0; i < decorationLayersCount; i++) {
-          addMarkerTextDecorationLayer(displayLayer, [])
+          addMarkerTextDecorationLayer(displayLayer, [], classNamesByMarkerId)
         }
         displayLayer.getText(0, 3)
         let undoableChanges = 0
@@ -2314,7 +2311,7 @@ describe('DisplayLayer', () => {
             const markerTextDecorationLayer = getRandomMarkerTextDecorationLayer(displayLayer, random)
             switch (random(3)) {
               case 0:
-                createRandomMarkerDecorations(buffer, markerTextDecorationLayer, random)
+                createRandomMarkerDecorations(buffer, markerTextDecorationLayer, classNamesByMarkerId, random)
                 break
               case 1:
                 moveRandomMarkerDecorations(buffer, markerTextDecorationLayer, random)
@@ -2713,13 +2710,17 @@ function getComputedScreenLineCount (displayLayer) {
   return displayLayer.screenLineLengths.length
 }
 
-function addMarkerTextDecorationLayer (displayLayer, decorations) {
+function addMarkerTextDecorationLayer (displayLayer, decorations, classNamesByMarkerId = new Map()) {
   const markerLayer = displayLayer.buffer.addMarkerLayer()
-  const decorationLayer = displayLayer.addMarkerTextDecorationLayer(markerLayer)
+  const decorationLayer = new MarkerTextDecorationLayer(
+    markerLayer,
+    {classNameForMarkerId: (id) => classNamesByMarkerId.get(id)}
+  )
   for (const [className, range] of decorations) {
     const marker = markerLayer.markRange(range)
-    decorationLayer.setClassNameForMarker(marker, className)
+    classNamesByMarkerId.set(marker.id, className)
   }
+  displayLayer.addTextDecorationLayer(decorationLayer)
   return decorationLayer
 }
 
@@ -2729,14 +2730,14 @@ function getRandomMarkerTextDecorationLayer (displayLayer, random) {
   return decorationLayers[decorationLayerIndex]
 }
 
-function createRandomMarkerDecorations (buffer, markerTextDecorationLayer, random) {
+function createRandomMarkerDecorations (buffer, markerTextDecorationLayer, classNamesByMarkerId, random) {
   log('create random marker decorations')
   for (var i = 0; i < random(10); i++) {
     const range = getRandomBufferRange(random, buffer)
     const invalidate = getRandomInvalidationStrategy(random)
     const marker = markerTextDecorationLayer.markerLayer.markRange(range, {invalidate})
     const className = WORDS[random(WORDS.length)]
-    markerTextDecorationLayer.setClassNameForMarker(marker, className)
+    classNamesByMarkerId.set(marker.id, className)
   }
 }
 

spec/marker-text-decoration-layer-spec.js

@@ -14,7 +14,7 @@ describe('MarkerTextDecorationLayer', () => {
       const marker3 = markerLayer.markRange([[0, 4], [1, 4]])
       const marker4 = markerLayer.markRange([[0, 4], [2, 3]])
 
-      const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer)
+      const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer, {})
       const iterator = textDecorationLayer.buildIterator()
       expect(iterator.seek(Point(0, 3))).toEqual([marker2.id])
       expect(iterator.getPosition()).toEqual(Point(0, 3))
@@ -60,7 +60,7 @@ describe('MarkerTextDecorationLayer', () => {
       expect(marker1.isValid()).toBe(false)
       const marker2 = markerLayer.markRange([[0, 4], [2, 3]])
 
-      const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer)
+      const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer, {})
       const iterator = textDecorationLayer.buildIterator()
       expect(iterator.seek(Point(0, 5))).toEqual([marker2.id])
       expect(iterator.getPosition()).toEqual(Point(1, 4))
@@ -83,7 +83,7 @@ describe('MarkerTextDecorationLayer', () => {
     const buffer = new TextBuffer({text: SAMPLE_TEXT})
     const markerLayer = buffer.addMarkerLayer()
     const marker1 = markerLayer.markRange([[0, 3], [1, 4]])
-    const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer)
+    const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer, {})
     let rangeInvalidationEvents
     textDecorationLayer.onDidInvalidateRange((range) => rangeInvalidationEvents.push(range))
 
@@ -123,32 +123,4 @@ describe('MarkerTextDecorationLayer', () => {
     textDecorationLayer.clearInvalidatedRanges()
     expect(textDecorationLayer.getInvalidatedRanges()).toEqual([])
   })
-
-  it('supports setting a default class name for all the markers in the underlying layer', () => {
-    const buffer = new TextBuffer({text: SAMPLE_TEXT})
-    const markerLayer = buffer.addMarkerLayer()
-    const marker = markerLayer.markRange([[0, 3], [1, 4]])
-    const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer)
-    expect(textDecorationLayer.classNameForScopeId(marker.id)).toBeNull()
-
-    textDecorationLayer.setDefaultClassName('default')
-    expect(textDecorationLayer.classNameForScopeId(marker.id)).toBe('default')
-
-    textDecorationLayer.setClassNameForMarker(marker, 'class-name')
-    expect(textDecorationLayer.classNameForScopeId(marker.id)).toBe('class-name')
-  })
-
-  it('supports setting a default inline style for all the markers in the underlying layer', () => {
-    const buffer = new TextBuffer({text: SAMPLE_TEXT})
-    const markerLayer = buffer.addMarkerLayer()
-    const marker = markerLayer.markRange([[0, 3], [1, 4]])
-    const textDecorationLayer = new MarkerTextDecorationLayer(markerLayer)
-    expect(textDecorationLayer.inlineStyleForScopeId(marker.id)).toBeNull()
-
-    textDecorationLayer.setDefaultInlineStyle({color: 'default'})
-    expect(textDecorationLayer.inlineStyleForScopeId(marker.id)).toEqual({color: 'default'})
-
-    textDecorationLayer.setInlineStyleForMarker(marker, {fontSize: '15px'})
-    expect(textDecorationLayer.inlineStyleForScopeId(marker.id)).toEqual({fontSize: '15px'})
-  })
 })

src/display-layer.js

@@ -172,12 +172,6 @@ class DisplayLayer {
     return markerLayer
   }
 
-  addMarkerTextDecorationLayer (markerLayer) {
-    const decorationLayer = new MarkerTextDecorationLayer(markerLayer)
-    this.addTextDecorationLayer(decorationLayer)
-    return decorationLayer
-  }
-
   getMarkerLayer (id) {
     if (this.displayMarkerLayersById.has(id)) {
       return this.displayMarkerLayersById.get(id)

src/marker-text-decoration-layer.js

@@ -1,41 +1,16 @@
 const {Emitter} = require('event-kit')
 const Point = require('./point')
+const NOOP = function () {}
 
 module.exports =
 class MarkerTextDecorationLayer {
-  constructor (markerLayer) {
+  constructor (markerLayer, {classNameForMarkerId, inlineStyleForMarkerId}) {
     this.markerLayer = markerLayer
     this.markerLayer.registerMarkerTextDecorationLayer(this)
-    this.classNamesByMarkerId = new Map()
-    this.inlineStylesByMarkerId = new Map()
     this.emitter = new Emitter()
     this.invalidatedRanges = []
-    this.defaultClassName = null
-    this.defaultInlineStyle = null
-  }
-
-  setDefaultClassName (className) {
-    this.defaultClassName = className
-  }
-
-  setClassNameForMarker (marker, className) {
-    this.classNamesByMarkerId.set(marker.id, className)
-  }
-
-  setDefaultInlineStyle (style) {
-    this.defaultInlineStyle = style
-  }
-
-  setInlineStyleForMarker (marker, style) {
-    this.inlineStylesByMarkerId.set(marker.id, style)
-  }
-
-  classNameForScopeId (markerId) {
-    return this.classNamesByMarkerId.get(markerId) || this.defaultClassName
-  }
-
-  inlineStyleForScopeId (markerId) {
-    return this.inlineStylesByMarkerId.get(markerId) || this.defaultInlineStyle
+    this.classNameForScopeId = classNameForMarkerId || NOOP
+    this.inlineStyleForScopeId = inlineStyleForMarkerId || NOOP
   }
 
   buildIterator () {

src/text-buffer.coffee

@@ -10,6 +10,7 @@ Point = require './point'
 Range = require './range'
 History = require './history'
 MarkerLayer = require './marker-layer'
+MarkerTextDecorationLayer = require './marker-text-decoration-layer'
 MatchIterator = require './match-iterator'
 DisplayLayer = require './display-layer'
 {spliceArray, newlineRegex, normalizePatchChanges, regexIsSingleLine} = require './helpers'
@@ -67,6 +68,7 @@ class TextBuffer
   @version: 5
   @Point: Point
   @Range: Range
+  @MarkerTextDecorationLayer: MarkerTextDecorationLayer
   @newlineRegex: newlineRegex
 
   cachedText: null