Merge pull request #262 from atom/multi-buffer-sharing

Allow guests to collaborate on multiple remote buffers (Part 1)

doc/rfcs/001-allow-guests-to-open-multiple-remote-buffers.md

@@ -2,7 +2,9 @@
 
 ## Status
 
-This is a proposal and is not yet implemented.
+This is a proposal and is not yet fully implemented.
+
+[#262](https://github.com/atom/teletype/pull/262) implements the majority of this proposal, and the remaining functionality is planned for future pull requests. See [#262](https://github.com/atom/teletype/pull/262) for details.
 
 ## Summary
 
@@ -32,7 +34,12 @@ When a host closes a buffer, it will be removed from all guest portals. If anoth
 
 You can follow any other guest participating in the host's workspace in the exact same way. If they move between buffers, you will follow them. The host does not enjoy any special privilege with respect to the ability to be followed between different files.
 
-When a participant is viewing a different buffer than you are viewing, that participant's avatar appears in the bottom right with an icon (e.g., https://octicons.github.com/icon/link-external) indicating that they're working on a different buffer.
+When viewing an editor associated with a portal, each participant sees the avatars for the other portal participants (just as they did prior to this RFC). As a host, when your active pane item is a local editor (i.e., an editor that you're sharing in your portal), the editor shows the avatars for the other portal participants. As a guest, when your active pane item is a remote editor (i.e., an editor that you're viewing from the host's portal), the editor shows the avatars for the other portal participants. The location of each avatar within the editor indicates the relative position of that participant:
+- Top-right: Participants in the same editor but in a row above your viewport
+- Middle-right: Participants in the same editor and inside your viewport
+- Bottom-right: Participants in the same editor but in a row below your viewport or a column outside of your viewport
+- Bottom-right with TBD icon 1: Participants in a different editor in the portal
+- Bottom-right with TBD icon 2: Participants in a different pane item not associated with the portal
 
 Editors for remote buffers are *only* automatically opened when you are following another collaborator. If you are not following someone, no editors are automatically opened. When you start following another collaborator again, an editor will be automatically opened based on their location. You can also open any buffer in the host's workspace directly by navigating to it...
 

lib/buffer-binding.js

@@ -4,13 +4,18 @@ function doNothing () {}
 
 module.exports =
 class BufferBinding {
-  constructor ({buffer, didDispose}) {
+  constructor ({buffer, isHost, didDispose}) {
     this.buffer = buffer
+    this.isHost = isHost
     this.emitDidDispose = didDispose || doNothing
     this.pendingChanges = []
+    this.disposed = false
   }
 
   dispose () {
+    if (this.disposed) return
+
+    this.disposed = true
     this.buffer.restoreDefaultHistoryProvider(this.bufferProxy.getHistory(this.buffer.maxUndoEntries))
     this.buffer = null
     if (this.bufferDestroySubscription) this.bufferDestroySubscription.dispose()
@@ -24,7 +29,13 @@ class BufferBinding {
       this.pushChange(this.pendingChanges.shift())
     }
     this.pendingChanges = null
-    this.bufferDestroySubscription = this.buffer.onDidDestroy(() => bufferProxy.dispose())
+    this.bufferDestroySubscription = this.buffer.onDidDestroy(() => {
+      if (this.isHost) {
+        bufferProxy.dispose()
+      } else {
+        this.dispose()
+      }
+    })
   }
 
   setText (text) {

lib/editor-binding.js

@@ -1,20 +1,17 @@
 /* global ResizeObserver */
 
 const path = require('path')
-const {Range, Disposable, CompositeDisposable} = require('atom')
+const {Range, Emitter, Disposable, CompositeDisposable} = require('atom')
 const normalizeURI = require('./normalize-uri')
 const {FollowState} = require('@atom/teletype-client')
-const SitePositionsComponent = require('./site-positions-component')
-
-function doNothing () {}
 
 module.exports =
 class EditorBinding {
-  constructor ({editor, portal, isHost, didDispose}) {
+  constructor ({editor, portal, isHost}) {
     this.editor = editor
     this.portal = portal
     this.isHost = isHost
-    this.emitDidDispose = didDispose || doNothing
+    this.emitter = new Emitter()
     this.selectionsMarkerLayer = this.editor.selectionsMarkerLayer.bufferMarkerLayer
     this.markerLayersBySiteId = new Map()
     this.markersByLayerAndId = new WeakMap()
@@ -24,18 +21,18 @@ class EditorBinding {
   }
 
   dispose () {
+    if (this.disposed) return
+
+    this.disposed = true
     this.subscriptions.dispose()
 
     this.markerLayersBySiteId.forEach((l) => l.destroy())
     this.markerLayersBySiteId.clear()
     if (!this.isHost) this.restoreOriginalEditorMethods(this.editor)
     if (this.localCursorLayerDecoration) this.localCursorLayerDecoration.destroy()
 
-    this.aboveViewportSitePositionsComponent.destroy()
-    this.insideViewportSitePositionsComponent.destroy()
-    this.outsideViewportSitePositionsComponent.destroy()
-
-    this.emitDidDispose()
+    this.emitter.emit('did-dispose')
+    this.emitter.dispose()
   }
 
   setEditorProxy (editorProxy) {
@@ -44,6 +41,7 @@ class EditorBinding {
       this.editor.onDidDestroy(() => this.editorProxy.dispose())
     } else {
       this.monkeyPatchEditorMethods(this.editor, this.editorProxy)
+      this.editor.onDidDestroy(() => this.dispose())
     }
 
     this.localCursorLayerDecoration = this.editor.decorateMarkerLayer(
@@ -59,15 +57,7 @@ class EditorBinding {
     this.subscriptions.add(this.editor.element.onDidChangeScrollTop(this.editorDidChangeScrollTop.bind(this)))
     this.subscriptions.add(this.editor.element.onDidChangeScrollLeft(this.editorDidChangeScrollLeft.bind(this)))
     this.subscriptions.add(subscribeToResizeEvents(this.editor.element, this.editorDidResize.bind(this)))
-    this.relayLocalSelections(true)
-
-    this.aboveViewportSitePositionsComponent = this.buildSitePositionsComponent('upper-right')
-    this.insideViewportSitePositionsComponent = this.buildSitePositionsComponent('middle-right')
-    this.outsideViewportSitePositionsComponent = this.buildSitePositionsComponent('lower-right')
-
-    this.editor.element.appendChild(this.aboveViewportSitePositionsComponent.element)
-    this.editor.element.appendChild(this.insideViewportSitePositionsComponent.element)
-    this.editor.element.appendChild(this.outsideViewportSitePositionsComponent.element)
+    this.relayLocalSelections()
   }
 
   monkeyPatchEditorMethods (editor, editorProxy) {
@@ -82,9 +72,13 @@ class EditorBinding {
     editor.copy = () => null
     editor.serialize = () => null
     editor.isRemote = true
+
+    let remoteEditorCountForBuffer = buffer.remoteEditorCount || 0
+    buffer.remoteEditorCount = ++remoteEditorCountForBuffer
     buffer.getPath = () => `${uriPrefix}:${bufferURI}`
     buffer.save = () => {}
     buffer.isModified = () => false
+
     editor.element.classList.add('teletype-RemotePaneItem')
   }
 
@@ -98,22 +92,27 @@ class EditorBinding {
     delete editor.copy
     delete editor.serialize
     delete editor.isRemote
-    delete buffer.getPath
-    delete buffer.save
-    delete buffer.isModified
+
+    buffer.remoteEditorCount--
+    if (buffer.remoteEditorCount === 0) {
+      delete buffer.remoteEditorCount
+      delete buffer.getPath
+      delete buffer.save
+      delete buffer.isModified
+    }
 
     editor.element.classList.remove('teletype-RemotePaneItem')
     editor.emitter.emit('did-change-title', editor.getTitle())
   }
 
-  observeMarker (marker, relayLocalSelections = true) {
+  observeMarker (marker, relay = true) {
     const didChangeDisposable = marker.onDidChange(({textChanged}) => {
       if (textChanged) {
         if (marker.getRange().isEmpty()) marker.clearTail()
       } else {
-        this.editorProxy.updateSelections({
+        this.updateSelections({
           [marker.id]: getSelectionState(marker)
-        }, this.preserveFollowState)
+        })
       }
     })
     const didDestroyDisposable = marker.onDidDestroy(() => {
@@ -122,33 +121,50 @@ class EditorBinding {
       this.subscriptions.remove(didChangeDisposable)
       this.subscriptions.remove(didDestroyDisposable)
 
-      this.editorProxy.updateSelections({
+      this.updateSelections({
         [marker.id]: null
-      }, this.preserveFollowState)
+      })
     })
     this.subscriptions.add(didChangeDisposable)
     this.subscriptions.add(didDestroyDisposable)
-    if (relayLocalSelections) this.relayLocalSelections()
+
+    if (relay) {
+      this.updateSelections({
+        [marker.id]: getSelectionState(marker)
+      })
+    }
   }
 
   async editorDidChangeScrollTop () {
     const {element} = this.editor
     await element.component.getNextUpdatePromise()
-    this.updateActivePositions(this.positionsBySiteId)
     this.editorProxy.didScroll()
+    this.emitter.emit('did-scroll')
   }
 
   async editorDidChangeScrollLeft () {
     const {element} = this.editor
     await element.component.getNextUpdatePromise()
-    this.updateActivePositions(this.positionsBySiteId)
     this.editorProxy.didScroll()
+    this.emitter.emit('did-scroll')
   }
 
   async editorDidResize () {
     const {element} = this.editor
     await element.component.getNextUpdatePromise()
-    this.updateActivePositions(this.positionsBySiteId)
+    this.emitter.emit('did-resize')
+  }
+
+  onDidDispose (callback) {
+    return this.emitter.on('did-dispose', callback)
+  }
+
+  onDidScroll (callback) {
+    return this.emitter.on('did-scroll', callback)
+  }
+
+  onDidResize (callback) {
+    return this.emitter.on('did-resize', callback)
   }
 
   updateSelectionsForSiteId (siteId, selections) {
@@ -198,19 +214,18 @@ class EditorBinding {
     }
   }
 
-  isPositionVisible (bufferPosition) {
-    return this.getDirectionFromViewportToPosition(bufferPosition) === 'inside'
+  isScrollNeededToViewPosition (position) {
+    const isPositionVisible = this.getDirectionFromViewportToPosition(position) === 'inside'
+    const isEditorAttachedToDOM = document.body.contains(this.editor.element)
+    return isEditorAttachedToDOM && !isPositionVisible
   }
 
   updateTether (state, position) {
     const localCursorDecorationProperties = {type: 'cursor'}
 
     if (state === FollowState.RETRACTED) {
       this.editor.destroyFoldsIntersectingBufferRange(Range(position, position))
-
-      this.preserveFollowState = true
-      this.editor.setCursorBufferPosition(position)
-      this.preserveFollowState = false
+      this.batchMarkerUpdates(() => this.editor.setCursorBufferPosition(position))
 
       localCursorDecorationProperties.style = {opacity: 0}
     } else {
@@ -220,36 +235,6 @@ class EditorBinding {
     this.localCursorLayerDecoration.setProperties(localCursorDecorationProperties)
   }
 
-  updateActivePositions (positionsBySiteId) {
-    const aboveViewportSiteIds = []
-    const insideViewportSiteIds = []
-    const outsideViewportSiteIds = []
-    const followedSiteId = this.editorProxy.getFollowedSiteId()
-
-    for (let siteId in positionsBySiteId) {
-      siteId = parseInt(siteId)
-      const position = positionsBySiteId[siteId]
-      switch (this.getDirectionFromViewportToPosition(position)) {
-        case 'upward':
-          aboveViewportSiteIds.push(siteId)
-          break
-        case 'inside':
-          insideViewportSiteIds.push(siteId)
-          break
-        case 'downward':
-        case 'leftward':
-        case 'rightward':
-          outsideViewportSiteIds.push(siteId)
-          break
-      }
-    }
-
-    this.aboveViewportSitePositionsComponent.update({siteIds: aboveViewportSiteIds, followedSiteId})
-    this.insideViewportSitePositionsComponent.update({siteIds: insideViewportSiteIds, followedSiteId})
-    this.outsideViewportSitePositionsComponent.update({siteIds: outsideViewportSiteIds, followedSiteId})
-    this.positionsBySiteId = positionsBySiteId
-  }
-
   getDirectionFromViewportToPosition (bufferPosition) {
     const {element} = this.editor
     if (!document.contains(element)) return
@@ -278,23 +263,31 @@ class EditorBinding {
     this.markersByLayerAndId.delete(markerLayer)
   }
 
-  relayLocalSelections (initialUpdate = false) {
+  relayLocalSelections () {
     const selectionUpdates = {}
     const selectionMarkers = this.selectionsMarkerLayer.getMarkers()
     for (let i = 0; i < selectionMarkers.length; i++) {
       const marker = selectionMarkers[i]
       selectionUpdates[marker.id] = getSelectionState(marker)
     }
-    this.editorProxy.updateSelections(selectionUpdates, initialUpdate)
+    this.editorProxy.updateSelections(selectionUpdates, {initialUpdate: true})
   }
 
-  buildSitePositionsComponent (position) {
-    return new SitePositionsComponent({
-      position,
-      displayedParticipantsCount: 3,
-      portal: this.portal,
-      onSelectSiteId: this.toggleFollowingForSiteId.bind(this)
-    })
+  batchMarkerUpdates (fn) {
+    this.batchedMarkerUpdates = {}
+    this.isBatchingMarkerUpdates = true
+    fn()
+    this.isBatchingMarkerUpdates = false
+    this.editorProxy.updateSelections(this.batchedMarkerUpdates)
+    this.batchedMarkerUpdates = null
+  }
+
+  updateSelections (update) {
+    if (this.isBatchingMarkerUpdates) {
+      Object.assign(this.batchedMarkerUpdates, update)
+    } else {
+      this.editorProxy.updateSelections(update)
+    }
   }
 
   toggleFollowingForSiteId (siteId) {