Merge pull request #353 from odoyle71/mod

[Code Submission] RFC-002: Sync Buffer Path Changes from Host To Guest

Author: jasonrudolph

doc/rfcs/002-sync-buffer-path-changes-from-host-to-guest.md

@@ -2,7 +2,7 @@
 
 ### Status
 
-Accepted
+Implemented. See [#352](https://github.com/atom/teletype/issues/352).
 
 ### Summary
 

lib/buffer-binding.js

@@ -1,4 +1,6 @@
-const {Range, TextBuffer} = require('atom')
+const {Emitter, Range, CompositeDisposable, TextBuffer} = require('atom')
+const getPathWithNativeSeparators = require('./get-path-with-native-separators')
+const path = require('path')
 
 function doNothing () {}
 
@@ -12,15 +14,21 @@ class BufferBinding {
     this.pendingChanges = []
     this.disposed = false
     this.disableHistory = false
+    this.subscriptions = new CompositeDisposable()
+    if (isHost) {
+      this.subscriptions.add(buffer.onDidChangePath(this.relayURIChange.bind(this)))
+    }
   }
 
   dispose () {
     if (this.disposed) return
 
     this.disposed = true
+    this.subscriptions.dispose()
     this.buffer.restoreDefaultHistoryProvider(this.bufferProxy.getHistory(this.buffer.maxUndoEntries))
     this.buffer = null
     if (this.bufferDestroySubscription) this.bufferDestroySubscription.dispose()
+    if (this.remoteFile) this.remoteFile.dispose()
     this.emitDidDispose()
   }
 
@@ -31,6 +39,10 @@ class BufferBinding {
       this.pushChange(this.pendingChanges.shift())
     }
     this.pendingChanges = null
+    if (!this.isHost) {
+      this.remoteFile = new RemoteFile({uri: bufferProxy.uri})
+      this.buffer.setFile(this.remoteFile)
+    }
     this.bufferDestroySubscription = this.buffer.onDidDestroy(() => {
       if (this.isHost) {
         bufferProxy.dispose()
@@ -152,6 +164,25 @@ class BufferBinding {
     if (this.buffer.getPath()) return this.saveBuffer()
   }
 
+  relayURIChange () {
+    this.bufferProxy.setURI(this.getBufferProxyURI())
+  }
+
+  didChangeURI (uri) {
+    if (this.remoteFile) this.remoteFile.setURI(uri)
+  }
+
+  getBufferProxyURI () {
+    if (!this.buffer.getPath()) return 'untitled'
+    const [projectPath, relativePath] = atom.workspace.project.relativizePath(this.buffer.getPath())
+    if (projectPath) {
+      const projectName = path.basename(projectPath)
+      return path.join(projectName, relativePath)
+    } else {
+      return relativePath
+    }
+  }
+
   serialize (options) {
     return this.serializeUsingDefaultHistoryProviderFormat(options)
   }
@@ -166,3 +197,31 @@ class BufferBinding {
     return serializedDefaultHistoryProvider
   }
 }
+
+class RemoteFile {
+  constructor ({uri}) {
+    this.uri = uri
+    this.emitter = new Emitter()
+  }
+
+  dispose () {
+    this.emitter.dispose()
+  }
+
+  getPath () {
+    return getPathWithNativeSeparators(this.uri)
+  }
+
+  setURI (uri) {
+    this.uri = uri
+    this.emitter.emit('did-rename')
+  }
+
+  onDidRename (callback) {
+    return this.emitter.on('did-rename', callback)
+  }
+
+  existsSync () {
+    return false
+  }
+}

lib/editor-binding.js

@@ -1,8 +1,7 @@
 /* global ResizeObserver */
 
 const path = require('path')
-const {Range, Emitter, Disposable, CompositeDisposable} = require('atom')
-const getPathWithNativeSeparators = require('./get-path-with-native-separators')
+const {Range, Emitter, Disposable, CompositeDisposable, TextBuffer} = require('atom')
 const {getEditorURI} = require('./uri-helpers')
 const {FollowState} = require('@atom/teletype-client')
 
@@ -58,23 +57,23 @@ class EditorBinding {
   }
 
   monkeyPatchEditorMethods (editor, editorProxy) {
-    const buffer = editor.getBuffer()
+    const remoteBuffer = editor.getBuffer()
+    const originalRemoteBufferGetPath = TextBuffer.prototype.getPath.bind(remoteBuffer)
     const {bufferProxy} = editorProxy
     const hostIdentity = this.portal.getSiteIdentity(1)
     const prefix = hostIdentity ? `@${hostIdentity.login}` : 'remote'
-    const bufferURI = getPathWithNativeSeparators(bufferProxy.uri)
 
-    editor.getTitle = () => `${prefix}: ${path.basename(bufferURI)}`
+    editor.getTitle = () => `${prefix}: ${path.basename(originalRemoteBufferGetPath())}`
     editor.getURI = () => getEditorURI(this.portal.id, editorProxy.id)
     editor.copy = () => null
     editor.serialize = () => null
     editor.isRemote = true
 
-    let remoteEditorCountForBuffer = buffer.remoteEditorCount || 0
-    buffer.remoteEditorCount = ++remoteEditorCountForBuffer
-    buffer.getPath = () => `${prefix}:${bufferURI}`
-    buffer.save = () => { bufferProxy.requestSave() }
-    buffer.isModified = () => false
+    let remoteEditorCountForBuffer = remoteBuffer.remoteEditorCount || 0
+    remoteBuffer.remoteEditorCount = ++remoteEditorCountForBuffer
+    remoteBuffer.getPath = () => `${prefix}:${originalRemoteBufferGetPath()}`
+    remoteBuffer.save = () => { bufferProxy.requestSave() }
+    remoteBuffer.isModified = () => false
 
     editor.element.classList.add('teletype-RemotePaneItem')
   }

lib/host-portal-binding.js

@@ -1,4 +1,3 @@
-const path = require('path')
 const {CompositeDisposable, Emitter} = require('atom')
 const {FollowState} = require('@atom/teletype-client')
 const BufferBinding = require('./buffer-binding')
@@ -145,7 +144,7 @@ class HostPortalBinding {
     } else {
       bufferBinding = new BufferBinding({buffer, isHost: true})
       const bufferProxy = this.portal.createBufferProxy({
-        uri: this.getBufferProxyURI(buffer),
+        uri: bufferBinding.getBufferProxyURI(),
         history: buffer.getHistory()
       })
       bufferBinding.setBufferProxy(bufferProxy)
@@ -156,16 +155,4 @@ class HostPortalBinding {
       return bufferProxy
     }
   }
-
-  getBufferProxyURI (buffer) {
-    if (!buffer.getPath()) return 'untitled'
-
-    const [projectPath, relativePath] = this.workspace.project.relativizePath(buffer.getPath())
-    if (projectPath) {
-      const projectName = path.basename(projectPath)
-      return path.join(projectName, relativePath)
-    } else {
-      return relativePath
-    }
-  }
 }

package.json

@@ -25,7 +25,7 @@
     "temp": "^0.8.3"
   },
   "dependencies": {
-    "@atom/teletype-client": "^0.36.1",
+    "@atom/teletype-client": "^0.37.0",
     "etch": "^0.12.6"
   },
   "providedServices": {

test/buffer-binding.test.js

@@ -1,16 +1,18 @@
 const assert = require('assert')
 const fs = require('fs')
+const path = require('path')
 const temp = require('temp')
-const {TextBuffer, Point} = require('atom')
+const {TextBuffer} = require('atom')
 const BufferBinding = require('../lib/buffer-binding')
+const FakeBufferProxy = require('./helpers/fake-buffer-proxy')
 
 suite('BufferBinding', function () {
   if (process.env.CI) this.timeout(process.env.TEST_TIMEOUT_IN_MS)
 
   test('relays changes to and from the shared buffer', () => {
     const buffer = new TextBuffer('hello\nworld')
     const binding = new BufferBinding({buffer})
-    const bufferProxy = new FakeBufferProxy(binding, buffer.getText())
+    const bufferProxy = new FakeBufferProxy({delegate: binding, text: buffer.getText()})
     binding.setBufferProxy(bufferProxy)
 
     bufferProxy.simulateRemoteTextUpdate([
@@ -36,7 +38,7 @@ suite('BufferBinding', function () {
   test('does not relay empty changes to the shared buffer', () => {
     const buffer = new TextBuffer('hello\nworld')
     const binding = new BufferBinding({buffer})
-    const bufferProxy = new FakeBufferProxy(binding, buffer.getText())
+    const bufferProxy = new FakeBufferProxy({delegate: binding, text: buffer.getText()})
     binding.setBufferProxy(bufferProxy)
 
     buffer.setTextInRange([[0, 0], [0, 0]], '')
@@ -49,8 +51,8 @@ suite('BufferBinding', function () {
     // This line ensures saving works correctly even if the save function has been monkey-patched.
     buffer.save = () => {}
 
-    const binding = new BufferBinding({buffer})
-    const bufferProxy = new FakeBufferProxy(binding, buffer.getText())
+    const binding = new BufferBinding({buffer, isHost: true})
+    const bufferProxy = new FakeBufferProxy({delegate: binding, text: buffer.getText()})
     binding.setBufferProxy(bufferProxy)
 
     // Calling binding.save with an in-memory buffer is ignored.
@@ -69,11 +71,34 @@ suite('BufferBinding', function () {
     assert.equal(fs.readFileSync(filePath, 'utf8'), 'changed')
   })
 
+  test('relays path changes from host to guest', async () => {
+    {
+      const hostBuffer = new TextBuffer('')
+      const hostBinding = new BufferBinding({buffer: hostBuffer, isHost: true})
+      const hostBufferProxy = new FakeBufferProxy({delegate: hostBinding, text: hostBuffer.getText()})
+      hostBinding.setBufferProxy(hostBufferProxy)
+
+      await hostBuffer.saveAs(path.join(temp.path(), 'new-filename'))
+      assert(hostBufferProxy.uri.includes('new-filename'))
+    }
+
+    {
+      const guestBuffer = new TextBuffer('')
+      const guestBinding = new BufferBinding({buffer: guestBuffer, isHost: false})
+      const guestBufferProxy = new FakeBufferProxy({delegate: guestBinding, text: guestBuffer.getText()})
+      guestBinding.setBufferProxy(guestBufferProxy)
+
+      guestBufferProxy.simulateRemoteURIChange('some/uri/new-filename')
+      assert(guestBuffer.getPath().includes('new-filename'))
+      assert.equal(guestBufferProxy.uri, 'some/uri/new-filename')
+    }
+  })
+
   suite('destroying the buffer', () => {
     test('on the host, disposes the underlying buffer proxy', () => {
       const buffer = new TextBuffer('')
       const binding = new BufferBinding({buffer, isHost: true})
-      const bufferProxy = new FakeBufferProxy(binding, buffer.getText())
+      const bufferProxy = new FakeBufferProxy({delegate: binding, text: buffer.getText()})
       binding.setBufferProxy(bufferProxy)
 
       buffer.destroy()
@@ -83,79 +108,12 @@ suite('BufferBinding', function () {
     test('on guests, disposes the buffer binding', () => {
       const buffer = new TextBuffer('')
       const binding = new BufferBinding({buffer, isHost: false})
-      const bufferProxy = new FakeBufferProxy(binding, buffer.getText())
+      const bufferProxy = new FakeBufferProxy({delegate: binding, text: buffer.getText()})
       binding.setBufferProxy(bufferProxy)
 
       buffer.destroy()
       assert(binding.disposed)
       assert(!bufferProxy.disposed)
     })
   })
-
-  class FakeBufferProxy {
-    constructor (delegate, text) {
-      this.delegate = delegate
-      this.text = text
-      this.disposed = false
-    }
-
-    dispose () {
-      this.disposed = true
-    }
-
-    getHistory () {
-      return {undoStack: [], redoStack: [], nextCheckpointId: 1}
-    }
-
-    setTextInRange (oldStart, oldEnd, newText) {
-      const oldStartIndex = characterIndexForPosition(this.text, oldStart)
-      const oldEndIndex = characterIndexForPosition(this.text, oldEnd)
-      this.text = this.text.slice(0, oldStartIndex) + newText + this.text.slice(oldEndIndex)
-    }
-
-    simulateRemoteTextUpdate (changes) {
-      assert(changes.length > 0, 'Must update text with at least one change')
-
-      for (let i = changes.length - 1; i >= 0; i--) {
-        const {oldStart, oldEnd, newText} = changes[i]
-        this.setTextInRange(oldStart, oldEnd, newText)
-      }
-
-      this.delegate.updateText(changes)
-    }
-
-    createCheckpoint () {
-      return 1
-    }
-
-    groupChangesSinceCheckpoint () {
-      return []
-    }
-
-    groupLastChanges () {
-      return true
-    }
-
-    applyGroupingInterval () {
-
-    }
-  }
-
-  function characterIndexForPosition (text, target) {
-    target = Point.fromObject(target)
-    const position = Point(0, 0)
-    let index = 0
-    while (position.compare(target) < 0 && index < text.length) {
-      if (text[index] === '\n') {
-        position.row++
-        position.column = 0
-      } else {
-        position.column++
-      }
-
-      index++
-    }
-
-    return index
-  }
 })

test/editor-binding.test.js

@@ -13,6 +13,8 @@ const {
   setEditorScrollLeftInChars
 } = require('./helpers/editor-helpers')
 const {FollowState} = require('@atom/teletype-client')
+const FakeBufferProxy = require('./helpers/fake-buffer-proxy')
+const BufferBinding = require('../lib/buffer-binding')
 
 suite('EditorBinding', function () {
   if (process.env.CI) this.timeout(process.env.TEST_TIMEOUT_IN_MS)
@@ -323,9 +325,11 @@ suite('EditorBinding', function () {
       const buffer = new TextBuffer({text: SAMPLE_TEXT})
       const editor = new TextEditor({buffer})
 
-      const binding = new EditorBinding({editor, portal: new FakePortal(), isHost: false})
-      const editorProxy = new FakeEditorProxy(binding)
-      binding.setEditorProxy(editorProxy)
+      const editorBinding = new EditorBinding({editor, portal: new FakePortal(), isHost: false})
+      const editorProxy = new FakeEditorProxy(editorBinding)
+      const bufferBinding = new BufferBinding({buffer})
+      bufferBinding.setBufferProxy(editorProxy.bufferProxy)
+      editorBinding.setEditorProxy(editorProxy)
       assert.equal(editor.getTitle(), '@site-1: fake-buffer-proxy-uri')
       assert.equal(editor.copy(), null)
       assert.equal(editor.serialize(), null)
@@ -458,13 +462,7 @@ suite('EditorBinding', function () {
 class FakeEditorProxy {
   constructor (delegate, {siteId} = {}) {
     this.delegate = delegate
-    this.bufferProxy = {
-      uri: 'fake-buffer-proxy-uri',
-      saveRequestCount: 0,
-      requestSave () {
-        this.saveRequestCount++
-      }
-    }
+    this.bufferProxy = new FakeBufferProxy({uri: 'fake-buffer-proxy-uri'})
     this.selections = {}
     this.siteId = (siteId == null) ? 1 : siteId
     this.disposed = false