Bladeren bron

Merge pull request #348 from sharelatex/as-fix-cm-share

Fix CodeMirror/ShareJS binding when switching docs
Alasdair Smith 8 jaren geleden
bovenliggende
commit
5e39e61c9b

+ 3 - 2
services/web/public/coffee/ide/editor/directives/cmEditor.coffee

@@ -17,8 +17,9 @@ define [
             attachToCM(sharejsDoc)
 
         attachToCM = (sharejsDoc) ->
-          cm.setValue(sharejsDoc.getSnapshot())
-          sharejsDoc.attachToCM(cm)
+          scope.$applyAsync () ->
+            Frontend.richText.openDoc(cm, sharejsDoc.getSnapshot())
+            sharejsDoc.attachToCM(cm)
 
         detachFromCM = (sharejsDoc) ->
           sharejsDoc.detachFromCM()

+ 1 - 10
services/web/public/coffee/ide/editor/sharejs/vendor/client/ace.coffee

@@ -87,15 +87,6 @@ window.sharejs.extendDoc 'attach_ace', (editor, keepEditorContents, maxDocLength
 
   editorDoc.on 'change', editorListener
 
-  # Listen for remote ops on the sharejs document
-  docListener = (op) ->
-    suppress = true
-    applyToDoc editorDoc, op
-    suppress = false
-
-    check()
-
-
   # Horribly inefficient.
   offsetToPos = (offset) ->
     # Again, very inefficient.
@@ -154,7 +145,7 @@ window.sharejs.extendDoc 'attach_ace', (editor, keepEditorContents, maxDocLength
     check()
 
   doc.detach_ace = ->
-    doc.removeListener 'remoteop', docListener
+    # TODO: can we remove the insert and delete event callbacks?
     editorDoc.removeListener 'change', editorListener
     delete doc.detach_ace
 

+ 5 - 4
services/web/public/coffee/ide/editor/sharejs/vendor/client/cm.coffee

@@ -36,6 +36,8 @@ window.sharejs.extendDoc 'attach_cm', (editor, keepEditorContents) ->
     throw new Error 'Only text documents can be attached to CodeMirror2'
 
   sharedoc = @
+  editorDoc = editor.getDoc()
+
   check = ->
     window.setTimeout ->
         editorText = editor.getValue()
@@ -64,11 +66,10 @@ window.sharejs.extendDoc 'attach_cm', (editor, keepEditorContents) ->
   # Listen for edits in CodeMirror.
   editorListener = (ed, change) ->
     return if suppress
-    applyCMToShareJS editor, change, sharedoc
-
+    applyCMToShareJS editorDoc, change, sharedoc
     check()
 
-  editor.on 'change', editorListener
+  editorDoc.on 'change', editorListener
 
   @on 'insert', (pos, text) ->
     suppress = true
@@ -87,7 +88,7 @@ window.sharejs.extendDoc 'attach_cm', (editor, keepEditorContents) ->
 
   @detach_cm = ->
     # TODO: can we remove the insert and delete event callbacks?
-    editor.off 'onChange', editorListener
+    editorDoc.off 'change', editorListener
     delete @detach_cm
 
   return

+ 7 - 1
services/web/public/es/rich-text.js

@@ -1,5 +1,11 @@
-import CodeMirror from 'codemirror'
+import CodeMirror, { Doc } from 'codemirror'
 
 export function init(rootEl) {
   return CodeMirror(rootEl)
 }
+
+export function openDoc(cm, content) {
+  const newDoc = Doc(content)
+  cm.swapDoc(newDoc)
+  return newDoc
+}

+ 9 - 6
services/web/test/unit_frontend/coffee/ide/editor/directives/cmEditorTests.coffee

@@ -4,9 +4,11 @@ define ['ide/editor/directives/cmEditor'], () ->
 
     beforeEach () ->
       @richTextInit = sinon.stub()
+      @richTextOpenDoc = sinon.stub()
       window.Frontend = {
         richText: {
-          init: @richTextInit
+          init: @richTextInit,
+          openDoc: @richTextOpenDoc
         }
       }
 
@@ -16,9 +18,7 @@ define ['ide/editor/directives/cmEditor'], () ->
         expect(@richTextInit).to.have.been.called
 
     it 'attaches to CM', () ->
-      inject ($compile, $rootScope) ->
-        setValue = sinon.stub()
-        @richTextInit.returns({ setValue: setValue })
+      inject ($compile, $rootScope, $browser) ->
         getSnapshot = sinon.stub()
         detachFromCM = sinon.stub()
         attachToCM = sinon.stub()
@@ -30,10 +30,13 @@ define ['ide/editor/directives/cmEditor'], () ->
 
         $compile('<div cm-editor sharejs-doc="sharejsDoc"></div>')($rootScope)
         $rootScope.$digest()
+        # Trigger $applyAsync to evaluate the expression, normally done in the
+        # next tick
+        $browser.defer.flush()
 
-        expect(getSnapshot).to.have.been.called
-        expect(setValue).to.have.been.called
         expect(detachFromCM).to.have.been.called
+        expect(getSnapshot).to.have.been.called
+        expect(@richTextOpenDoc).to.have.been.called
         expect(attachToCM).to.have.been.called
 
     it 'detaches from CM when destroyed', () ->