HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Realtime collaborative editor: a CodeMirror extension for MobWrite

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codemirroreditormobwriteextensioncollaborativeforrealtime

Problem

I wrote up the following script for the realtime synchronization service, MobWrite, to be used with the browser-based editor, CodeMirror:

```
// Based on MobWrite's code for TextBoxes
/*Instructions for use:
Set an 'id' property for your codeMirror obj
import this code after you import MobWrite.
*/

//Licence: http://creativecommons.org/licenses/by-nc-sa/3.0/

function linech2n(ed, linech) {
var line = linech.line;
var ch = linech.ch;
var n = line + ch; //for the \n s & chars in the line
for(i=0;i} patches Array of Patch objects.
*/
mobwrite.shareCodeMirrorObj.prototype.patchClientText = function(patches) {
// Set some constants which tweak the matching behaviour.
// Maximum distance to search from expected location.
this.dmp.Match_Distance = 1000;
// At what point is no match declared (0.0 = perfection, 1.0 = very loose)
this.dmp.Match_Threshold = 0.6;

var oldClientText = this.getClientText();
var cursor = this.captureCursor_();
// Pack the cursor offsets into an array to be adjusted.
// See http://neil.fraser.name/writing/cursor/
var offsets = [];
if (cursor) {
offsets[0] = cursor.startOffset;
if ('endOffset' in cursor) {
offsets[1] = cursor.endOffset;
}
}
var newClientText = this.patch_apply_(patches, oldClientText, offsets);
// Set the new text only if there is a change to be made.
if (oldClientText != newClientText) {
this.setClientText(newClientText);
if (cursor) {
// Unpack the offset array.
cursor.startOffset = offsets[0];
if (offsets.length > 1) {
cursor.endOffset = offsets[1];
if (cursor.startOffset >= cursor.endOffset) {
cursor.collapsed = true;
}
}
this.restoreCursor_(cursor);
}
}
};

/**
* Merge a set of patches onto the text. Return a patched text.
* @param {Array.} patches Array of patch objects.
* @param {string} text Old text.
* @param {Array.} offsets Offset indices to adjust.
* @return {string} New

Solution

From a once over:

  • You should remove commented out code



  • both linech2n and n2linech could use better naming of the function and variables, and a line of comment.



  • one liner if statements like if(!"id" in cmObj) cmObj.id="TEMP"; should be split in 2 lines



  • mobwrite.shareCodeMirrorObj.prototype = new mobwrite.shareObj(''); should probably be captured into an init function instead of being jammed between function declarations



  • mobwrite.shareCodeMirrorObj.prototype.patch_apply_ has some weird indenting on top



  • lowerCamelCasing is good for you : expected_loc, start_loc, begin_loc etc. should get lowerCamelCased.



-
This:

// Deal with loose ends
if (cursorStartPoint === null && cursorEndPoint !== null) {
  // Lost the start point of the selection, but we have the end point.
  // Collapse to end point.
  cursorStartPoint = cursorEndPoint;
} else if (cursorStartPoint === null && cursorEndPoint === null) {
  // Lost both start and end points.
  // Jump to the offset of start.
  cursorStartPoint = cursor.startOffset;
}
if (cursorEndPoint === null) {
  // End not known, collapse to start.
  cursorEndPoint = cursorStartPoint;
}


could be

//Try to determine the start & end point
cursorStartPoint = cursorStartPoint || cursorEndPoint || cursor.startOffset;
cursorEndPoint   = cursorEndPoint || cursorStartPoint;

Code Snippets

// Deal with loose ends
if (cursorStartPoint === null && cursorEndPoint !== null) {
  // Lost the start point of the selection, but we have the end point.
  // Collapse to end point.
  cursorStartPoint = cursorEndPoint;
} else if (cursorStartPoint === null && cursorEndPoint === null) {
  // Lost both start and end points.
  // Jump to the offset of start.
  cursorStartPoint = cursor.startOffset;
}
if (cursorEndPoint === null) {
  // End not known, collapse to start.
  cursorEndPoint = cursorStartPoint;
}
//Try to determine the start & end point
cursorStartPoint = cursorStartPoint || cursorEndPoint || cursor.startOffset;
cursorEndPoint   = cursorEndPoint || cursorStartPoint;

Context

StackExchange Code Review Q#9444, answer score: 3

Revisions (0)

No revisions yet.