Refactor ops model so it all happens in Document

services/web/app/views/project/editor/editor.jade

@@ -53,7 +53,6 @@ div.full-size(
 			events-bridge="reviewPanelEventsBridge"
 			track-changes-enabled="trackChangesFeatureFlag",
 			track-changes= "editor.trackChanges",
-			ranges-tracker="reviewPanel.rangesTracker",
 			doc-id="editor.open_doc_id"
 		)
 		

services/web/public/coffee/ide/editor/Document.coffee

@@ -1,7 +1,8 @@
 define [
 	"utils/EventEmitter"
 	"ide/editor/ShareJsDoc"
-], (EventEmitter, ShareJsDoc) ->
+	"ide/review-panel/RangesTracker"
+], (EventEmitter, ShareJsDoc, RangesTracker) ->
 	class Document extends EventEmitter
 		@getDocument: (ide, doc_id) ->
 			@openDocs ||= {}
@@ -247,14 +248,15 @@ define [
 					@joined = true
 					@doc.catchUp( updates )
 					# TODO: Worry about whether these ranges are consistent with the doc still
-					@opening_ranges = ranges
+					@ranges?.changes = ranges?.changes
+					@ranges?.comments = ranges?.comments
 					callback()
 			else
 				@ide.socket.emit 'joinDoc', @doc_id, (error, docLines, version, updates, ranges) =>
 					return callback(error) if error?
 					@joined = true
 					@doc = new ShareJsDoc @doc_id, docLines, version, @ide.socket
-					@opening_ranges = ranges
+					@ranges = new RangesTracker(ranges?.changes, ranges?.comments)
 					@_bindToShareJsDocEvents()
 					callback()
 
@@ -313,6 +315,8 @@ define [
 					inflightOp: inflightOp,
 					pendingOp: pendingOp
 					v: version
+			@doc.on "change", (ops, oldSnapshot, msg) =>
+				@_applyOpsToRanges(ops, oldSnapshot, msg)
 
 		_onError: (error, meta = {}) ->
 			meta.doc_id = @doc_id
@@ -325,3 +329,16 @@ define [
 			# the disconnect event, which means we try to leaveDoc when the connection comes back.
 			# This could intefere with the new connection of a new instance of this document.
 			@_cleanUp()
+
+		_applyOpsToRanges: (ops = [], oldSnapshot, msg) ->
+			track_changes_as = null
+			remote_op = msg?
+			if remote_op and msg.meta?.tc
+				track_changes_as = msg.meta.user_id
+			else if !remote_op and @track_changes_as?
+				track_changes_as = @track_changes_as
+			console.log "CHANGED", oldSnapshot, ops, track_changes_as
+			@ranges.track_changes = track_changes_as?
+			for op in ops
+				console.log "APPLYING OP", op, @ranges.track_changes 
+				@ranges.applyOp op, { user_id: track_changes_as }

services/web/public/coffee/ide/editor/ShareJsDoc.coffee

@@ -34,8 +34,8 @@ define [
 			@_doc = new ShareJs.Doc @connection, @doc_id,
 				type: @type
 			@_doc.setFlushDelay(SINGLE_USER_FLUSH_DELAY)
-			@_doc.on "change", () =>
-				@trigger "change"
+			@_doc.on "change", (args...) =>
+				@trigger "change", args...
 			@_doc.on "acknowledge", () =>
 				@lastAcked = new Date() # note time of last ack from server for an op we sent
 				@trigger "acknowledge"

services/web/public/coffee/ide/editor/directives/aceEditor.coffee

@@ -56,7 +56,6 @@ define [
 				eventsBridge: "="
 				trackChanges: "="
 				trackChangesEnabled: "="
-				rangesTracker: "="
 				docId: "="
 			}
 			link: (scope, element, attrs) ->

services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee

@@ -10,22 +10,15 @@ define [
 		constructor: (@$scope, @editor, @element) ->
 			window.trackChangesManager ?= @
 
-			@$scope.$watch "rangesTracker", (rangesTracker) =>
-				return if !rangesTracker?
-				@disconnectFromRangesTracker()
-				@rangesTracker = rangesTracker
-				@connectToRangesTracker()
-
 			@$scope.$watch "trackChanges", (track_changes) =>
 				return if !track_changes?
-				@rangesTracker?.track_changes = track_changes
+				@setTrackChanges(track_changes)
 			
 			@$scope.$watch "sharejsDoc", (doc) =>
 				return if !doc?
-				if doc.opening_ranges?.changes?
-					@rangesTracker.changes = doc.opening_ranges.changes
-				if doc.opening_ranges?.comments?
-					@rangesTracker.comments = doc.opening_ranges.comments
+				@disconnectFromRangesTracker()
+				@rangesTracker = doc.ranges
+				@connectToRangesTracker()
 			
 			@$scope.$on "comment:add", (e, comment) =>
 				@addCommentToSelection(comment)
@@ -65,48 +58,15 @@ define [
 			onResize = () =>
 				@recalculateReviewEntriesScreenPositions()
 
-			onChange = (e) =>
-				if !@editor.initing
-					# This change is trigger by a sharejs 'change' event, which is before the
-					# sharejs 'remoteop' event. So wait until the next event loop when the 'remoteop'
-					# will have fired, before we decide if it was a remote op.
-					setTimeout () =>
-						if @nextUpdateMetaData?
-							user_id = @nextUpdateMetaData.user_id
-							# The remote op may have contained multiple atomic ops, each of which is an Ace
-							# 'change' event (i.e. bulk commenting out of lines is a single remote op
-							# but gives us one event for each % inserted). These all come in a single event loop
-							# though, so wait until the next one before clearing the metadata.
-							setTimeout () =>
-								@nextUpdateMetaData = null
-						else
-							user_id = window.user.id
-						
-						was_tracking = @rangesTracker.track_changes
-						if @dont_track_next_update
-							@rangesTracker.track_changes = false
-							@dont_track_next_update = false
-						@applyChange(e, { user_id })
-						@rangesTracker.track_changes = was_tracking
-						
-						# TODO: Just for debugging, remove before going live.
-						setTimeout () =>
-							@checkMapping()
-						, 100
-
 			onChangeSession = (e) =>
-				e.oldSession?.getDocument().off "change", onChange
-				e.session.getDocument().on "change", onChange
 				@redrawAnnotations()
 
 			bindToAce = () =>
-				@editor.getSession().getDocument().on "change", onChange
 				@editor.on "changeSelection", onChangeSelection
 				@editor.on "changeSession", onChangeSession
 				@editor.renderer.on "resize", onResize
 
 			unbindFromAce = () =>
-				@editor.getSession().getDocument().off "change", onChange
 				@editor.off "changeSelection", onChangeSelection
 				@editor.off "changeSession", onChangeSession
 				@editor.renderer.off "resize", onResize
@@ -132,41 +92,49 @@ define [
 				@rangesTracker.off "comment:removed"
 				@rangesTracker.off "comment:resolved"
 				@rangesTracker.off "comment:unresolved"
+				
+		setTrackChanges: (value) ->
+			if value
+				@$scope.sharejsDoc?.track_changes_as = window.user.id
+			else
+				@$scope.sharejsDoc?.track_changes_as = null
 		
 		connectToRangesTracker: () ->
-			@rangesTracker.track_changes = @$scope.trackChanges
+			@setTrackChanges(@$scope.trackChanges)
 			
+			# Add a timeout because on remote ops, we get these notifications before
+			# ace has updated
 			@rangesTracker.on "insert:added", (change) =>
 				sl_console.log "[insert:added]", change
-				@_onInsertAdded(change)
+				setTimeout () => @_onInsertAdded(change)
 			@rangesTracker.on "insert:removed", (change) =>
 				sl_console.log "[insert:removed]", change
-				@_onInsertRemoved(change)
+				setTimeout () => @_onInsertRemoved(change)
 			@rangesTracker.on "delete:added", (change) =>
 				sl_console.log "[delete:added]", change
-				@_onDeleteAdded(change)
+				setTimeout () => @_onDeleteAdded(change)
 			@rangesTracker.on "delete:removed", (change) =>
 				sl_console.log "[delete:removed]", change
-				@_onDeleteRemoved(change)
+				setTimeout () => @_onDeleteRemoved(change)
 			@rangesTracker.on "changes:moved", (changes) =>
 				sl_console.log "[changes:moved]", changes
-				@_onChangesMoved(changes)
+				setTimeout () => @_onChangesMoved(changes)
 
 			@rangesTracker.on "comment:added", (comment) =>
 				sl_console.log "[comment:added]", comment
-				@_onCommentAdded(comment)
+				setTimeout () => @_onCommentAdded(comment)
 			@rangesTracker.on "comment:moved", (comment) =>
 				sl_console.log "[comment:moved]", comment
-				@_onCommentMoved(comment)
+				setTimeout () => @_onCommentMoved(comment)
 			@rangesTracker.on "comment:removed", (comment) =>
 				sl_console.log "[comment:removed]", comment
-				@_onCommentRemoved(comment)
+				setTimeout () => @_onCommentRemoved(comment)
 			@rangesTracker.on "comment:resolved", (comment) =>
 				sl_console.log "[comment:resolved]", comment
-				@_onCommentRemoved(comment)
+				setTimeout () => @_onCommentRemoved(comment)
 			@rangesTracker.on "comment:unresolved", (comment) =>
 				sl_console.log "[comment:unresolved]", comment
-				@_onCommentAdded(comment)
+				setTimeout () => @_onCommentAdded(comment)
 			
 		redrawAnnotations: () ->
 			for change in @rangesTracker.changes

services/web/public/coffee/ide/editor/sharejs/vendor/client/doc.coffee

@@ -71,7 +71,7 @@ class Doc
     # Its important that these event handlers are called with oldSnapshot.
     # The reason is that the OT type APIs might need to access the snapshots to
     # determine information about the received op.
-    @emit 'change', docOp, oldSnapshot
+    @emit 'change', docOp, oldSnapshot, msg
     @emit 'remoteop', docOp, oldSnapshot, msg if isRemote
   
   _connectionStateChanged: (state, data) ->
@@ -274,6 +274,7 @@ class Doc
   submitOp: (op, callback) ->
     op = @type.normalize(op) if @type.normalize?
 
+    oldSnapshot = @snapshot
     # If this throws an exception, no changes should have been made to the doc
     @snapshot = @type.apply @snapshot, op
 
@@ -284,7 +285,7 @@ class Doc
 
     @pendingCallbacks.push callback if callback
 
-    @emit 'change', op
+    @emit 'change', op, oldSnapshot
 
     @delayedFlush()
 

services/web/public/coffee/ide/editor/sharejs/vendor/types/text.coffee

@@ -31,7 +31,8 @@ checkValidComponent = (c) ->
 
   i_type = typeof c.i
   d_type = typeof c.d
-  throw new Error 'component needs an i or d field' unless (i_type == 'string') ^ (d_type == 'string')
+  c_type = typeof c.c
+  throw new Error 'component needs an i, d or c field' unless (i_type == 'string') ^ (d_type == 'string') ^ (c_type == 'string')
 
   throw new Error 'position cannot be negative' unless c.p >= 0
 
@@ -44,11 +45,15 @@ text.apply = (snapshot, op) ->
   for component in op
     if component.i?
       snapshot = strInject snapshot, component.p, component.i
-    else
+    else if component.d?
       deleted = snapshot[component.p...(component.p + component.d.length)]
       throw new Error "Delete component '#{component.d}' does not match deleted text '#{deleted}'" unless component.d == deleted
       snapshot = snapshot[...component.p] + snapshot[(component.p + component.d.length)..]
-  
+    else if component.c?
+      comment = snapshot[component.p...(component.p + component.c.length)]
+      throw new Error "Comment component '#{component.c}' does not match commented text '#{comment}'" unless component.c == comment
+    else
+      throw new Error "Unknown op type"
   snapshot
 
 
@@ -112,7 +117,7 @@ transformPosition = (pos, c, insertAfter) ->
       pos + c.i.length
     else
       pos
-  else
+  else if c.d?
     # I think this could also be written as: Math.min(c.p, Math.min(c.p - otherC.p, otherC.d.length))
     # but I think its harder to read that way, and it compiles using ternary operators anyway
     # so its no slower written like this.
@@ -122,6 +127,10 @@ transformPosition = (pos, c, insertAfter) ->
       c.p
     else
       pos - c.d.length
+  else if c.c?
+    pos
+  else
+    throw new Error("unknown op type")
 
 # Helper method to transform a cursor position as a result of an op.
 #
@@ -143,7 +152,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
   if c.i?
     append dest, {i:c.i, p:transformPosition(c.p, otherC, side == 'right')}
 
-  else # Delete
+  else if c.d? # Delete
     if otherC.i? # delete vs insert
       s = c.d
       if c.p < otherC.p
@@ -152,7 +161,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
       if s != ''
         append dest, {d:s, p:c.p + otherC.i.length}
 
-    else # Delete vs delete
+    else if otherC.d? # Delete vs delete
       if c.p >= otherC.p + otherC.d.length
         append dest, {d:c.d, p:c.p - otherC.d.length}
       else if c.p + c.d.length <= otherC.p
@@ -177,6 +186,51 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
           # This could be rewritten similarly to insert v delete, above.
           newC.p = transformPosition newC.p, otherC
           append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
+
+  else if c.c? # Comment
+    if otherC.i?
+      if c.p < otherC.p < c.p + c.c.length
+        offset = otherC.p - c.p
+        new_c = (c.c[0..(offset-1)] + otherC.i + c.c[offset...])
+        append dest, {c:new_c, p:c.p, t: c.t}
+      else
+        append dest, {c:c.c, p:transformPosition(c.p, otherC, true), t: c.t}
+    
+    else if otherC.d?
+      if c.p >= otherC.p + otherC.d.length
+        append dest, {c:c.c, p:c.p - otherC.d.length, t: c.t}
+      else if c.p + c.c.length <= otherC.p
+        append dest, c
+      else # Delete overlaps comment
+        # They overlap somewhere.
+        newC = {c:'', p:c.p, t: c.t}
+        if c.p < otherC.p
+          newC.c = c.c[...(otherC.p - c.p)]
+        if c.p + c.c.length > otherC.p + otherC.d.length
+          newC.c += c.c[(otherC.p + otherC.d.length - c.p)..]
+
+        # This is entirely optional - just for a check that the deleted
+        # text in the two ops matches
+        intersectStart = Math.max c.p, otherC.p
+        intersectEnd = Math.min c.p + c.c.length, otherC.p + otherC.d.length
+        cIntersect = c.c[intersectStart - c.p...intersectEnd - c.p]
+        otherIntersect = otherC.d[intersectStart - otherC.p...intersectEnd - otherC.p]
+        throw new Error 'Delete ops delete different text in the same region of the document' unless cIntersect == otherIntersect
+
+        newC.p = transformPosition newC.p, otherC
+        append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
   
   dest
 

services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee

@@ -154,10 +154,13 @@ define [
 			if view == $scope.SubViews.OVERVIEW
 				refreshOverviewPanel()
 
-		$scope.$watch "editor.open_doc_id", (open_doc_id) ->
-			return if !open_doc_id?
-			rangesTrackers[open_doc_id] ?= new RangesTracker()
-			$scope.reviewPanel.rangesTracker = rangesTrackers[open_doc_id]
+		$scope.$watch "editor.sharejs_doc", (doc) ->
+			return if !doc?
+			console.log "DOC changed", doc
+			# The open doc range tracker is kept up to date in real-time so
+			# replace any outdated info with this
+			rangesTrackers[doc.doc_id] = doc.ranges
+			$scope.reviewPanel.rangesTracker = rangesTrackers[doc.doc_id]
 
 		$scope.$watch (() ->
 			entries = $scope.reviewPanel.entries[$scope.editor.open_doc_id] or {}