diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index a0c07443c8..1287f207a5 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -41,6 +41,8 @@ define [ editorDoc = @ace?.getSession().getDocument() editorDoc?.off "change", @_checkConsistency @ide.$scope.$emit 'document:closed', @doc + + submitOp: (args...) -> @doc?.submitOp(args...) _checkConsistency: () -> # We've been seeing a lot of errors when I think there shouldn't be diff --git a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee index feb82c1e3f..48a7bbf3c6 100644 --- a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee +++ b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee @@ -61,6 +61,7 @@ define [ @_doc._onMessage message catch error # Version mismatches are thrown as errors + console.log error @_handleError(error) if message?.meta?.type == "external" diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee index ca8452f5c0..f8aa9099f1 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee @@ -20,8 +20,8 @@ define [ @rangesTracker = doc.ranges @connectToRangesTracker() - @$scope.$on "comment:add", (e, comment) => - @addCommentToSelection(comment) + @$scope.$on "comment:add", (e) => + @addCommentToSelection() @$scope.$on "comment:select_line", (e) => @selectLineIfNoSelection() @@ -146,21 +146,16 @@ define [ for comment in @rangesTracker.comments @_onCommentAdded(comment) - addComment: (offset, length, content) -> - @rangesTracker.addComment offset, length, { - thread: [{ - content: content - user_id: window.user_id - ts: new Date() - }] - } + addComment: (offset, content) -> + op = { c: content, p: offset } + # @rangesTracker.applyOp op # Will apply via sharejs + @$scope.sharejsDoc.submitOp op - addCommentToSelection: (content) -> + addCommentToSelection: () -> range = @editor.getSelectionRange() + content = @editor.getSelectedText() offset = @_aceRangeToShareJs(range.start) - end = @_aceRangeToShareJs(range.end) - length = end - offset - @addComment(offset, length, content) + @addComment(offset, content) selectLineIfNoSelection: () -> if @editor.selection.isEmpty() @@ -201,6 +196,7 @@ define [ @rangesTracker.unresolveCommentId(comment_id) checkMapping: () -> + # TODO: reintroduce this check session = @editor.getSession() # Make a copy of session.getMarkers() so we can modify it @@ -224,8 +220,8 @@ define [ for comment in @rangesTracker.comments if @changeIdToMarkerIdMap[comment.id]? {background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[comment.id] - start = @_shareJsOffsetToAcePosition(comment.offset) - end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + start = @_shareJsOffsetToAcePosition(comment.op.p) + end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length) expected_markers.push { marker_id: background_marker_id, start, end } expected_markers.push { marker_id: callout_marker_id, start, end: start } @@ -341,8 +337,8 @@ define [ _onCommentAdded: (comment) -> if !@changeIdToMarkerIdMap[comment.id]? # Only create new markers if they don't already exist - start = @_shareJsOffsetToAcePosition(comment.offset) - end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + start = @_shareJsOffsetToAcePosition(comment.op.p) + end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length) session = @editor.getSession() doc = session.getDocument() background_range = new Range(start.row, start.column, end.row, end.column) @@ -387,8 +383,8 @@ define [ @broadcastChange() _onCommentMoved: (comment) -> - start = @_shareJsOffsetToAcePosition(comment.offset) - end = @_shareJsOffsetToAcePosition(comment.offset + comment.length) + start = @_shareJsOffsetToAcePosition(comment.op.p) + end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length) @_updateMarker(comment.id, start, end) @editor.renderer.updateBackMarkers() @broadcastChange() diff --git a/services/web/public/coffee/ide/editor/sharejs/vendor/types/text-api.coffee b/services/web/public/coffee/ide/editor/sharejs/vendor/types/text-api.coffee index 96243ceffb..274b6019c5 100644 --- a/services/web/public/coffee/ide/editor/sharejs/vendor/types/text-api.coffee +++ b/services/web/public/coffee/ide/editor/sharejs/vendor/types/text-api.coffee @@ -28,5 +28,5 @@ text.api = for component in op if component.i != undefined @emit 'insert', component.p, component.i - else + else if component.d != undefined @emit 'delete', component.p, component.d diff --git a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee index 6a3625fd09..550c7da585 100644 --- a/services/web/public/coffee/ide/review-panel/RangesTracker.coffee +++ b/services/web/public/coffee/ide/review-panel/RangesTracker.coffee @@ -48,15 +48,6 @@ load = (EventEmitter) -> # sync with Ace ranges. @id = 0 - addComment: (offset, length, metadata) -> - # TODO: Don't allow overlapping comments? - @comments.push comment = { - id: @_newId() - offset, length, metadata - } - @emit "comment:added", comment - return comment - getComment: (comment_id) -> comment = null for c in @comments @@ -97,7 +88,7 @@ load = (EventEmitter) -> return if !change? @_removeChange(change) - applyOp: (op, metadata) -> + applyOp: (op, metadata = {}) -> metadata.ts ?= new Date() # Apply an op that has been applied to the document to our changes to keep them up to date if op.i? @@ -106,14 +97,32 @@ load = (EventEmitter) -> else if op.d? @applyDeleteToChanges(op, metadata) @applyDeleteToComments(op) + else if op.c? + @addComment(op, metadata) + else + throw new Error("unknown op type") + + addComment: (op, metadata) -> + # TODO: Don't allow overlapping comments? + @comments.push comment = { + id: @_newId() + op: # Copy because we'll modify in place + c: op.c + p: op.p + t: op.t + metadata + } + @emit "comment:added", comment + return comment applyInsertToComments: (op) -> for comment in @comments - if op.p <= comment.offset - comment.offset += op.i.length + if op.p <= comment.op.p + comment.op.p += op.i.length @emit "comment:moved", comment - else if op.p < comment.offset + comment.length - comment.length += op.i.length + else if op.p < comment.op.p + comment.op.c.length + offset = op.p - comment.op.p + comment.op.c = comment.op.c[0..(offset-1)] + op.i + comment.op.c[offset...] @emit "comment:moved", comment applyDeleteToComments: (op) -> @@ -121,20 +130,35 @@ load = (EventEmitter) -> op_length = op.d.length op_end = op.p + op_length for comment in @comments - comment_end = comment.offset + comment.length - if op_end <= comment.offset + comment_start = comment.op.p + comment_end = comment.op.p + comment.op.c.length + comment_length = comment_end - comment_start + if op_end <= comment_start # delete is fully before comment - comment.offset -= op_length + comment.op.p -= op_length @emit "comment:moved", comment else if op_start >= comment_end # delete is fully after comment, nothing to do else # delete and comment overlap - delete_length_before = Math.max(0, comment.offset - op_start) - delete_length_after = Math.max(0, op_end - comment_end) - delete_length_overlapping = op_length - delete_length_before - delete_length_after - comment.offset = Math.min(comment.offset, op_start) - comment.length -= delete_length_overlapping + if op_start <= comment_start + remaining_before = "" + else + remaining_before = comment.op.c.slice(0, op_start - comment_start) + if op_end >= comment_end + remaining_after = "" + else + remaining_after = comment.op.c.slice(op_end - comment_start) + + # Check deleted content matches delete op + deleted_comment = comment.op.c.slice(remaining_before.length, comment_length - remaining_after.length) + offset = Math.max(0, comment_start - op_start) + deleted_op_content = op.d.slice(offset).slice(0, deleted_comment.length) + if deleted_comment != deleted_op_content + throw new Error("deleted content does not match comment content") + + comment.op.p = Math.min(comment_start, op_start) + comment.op.c = remaining_before + remaining_after @emit "comment:moved", comment applyInsertToChanges: (op, metadata) -> diff --git a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee index df4afe306a..cc48079b8e 100644 --- a/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee +++ b/services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee @@ -218,11 +218,11 @@ define [ entries[comment.id] ?= {} new_entry = { type: "comment" - thread: comment.metadata.thread - resolved: comment.metadata.resolved - resolved_data: comment.metadata.resolved_data - offset: comment.offset - length: comment.length + thread: comment.metadata.thread or [] + resolved: comment.metadata?.resolved + resolved_data: comment.metadata?.resolved_data + content: comment.op.c + offset: comment.op.p } for key, value of new_entry entries[comment.id][key] = value