mizzao / meteor-sharejs

Meteor smart package for transparently adding ShareJS editors to an app
MIT License
225 stars 53 forks source link

Ace editor 1.2.0 is compatible with ShareJS 0.6 #55

Closed DavidSichau closed 8 years ago

DavidSichau commented 8 years ago

It seems that in ace 1.2 some changes were made which made ace sharejs incompatible:

https://github.com/ajaxorg/ace/pull/1745

Here is the stacktrace:

Uncaught TypeError: Cannot read property 'range' of undefinedapplyToShareJS @ ace.js:23
window.sharejs.extendDoc.editorListener @ ace.js:78
EventEmitter._signal @ ace.js:3378
module.applyDelta @ ace.js:6476
module.insertMergedLines @ ace.js:6374
module.insert @ ace.js:6302
module.insert @ ace.js:8817
module.insert @ ace.js:11961
exports.commands.exec @ ace.js:11105
(anonymous function) @ ace.js:10441
EventEmitter._emit.EventEmitter._dispatchEvent @ ace.js:3368
module.exec @ ace.js:10469
module.onTextInput @ ace.js:4091
module.onTextInput @ ace.js:11987
sendText @ ace.js:2117
onInput @ ace.js:2126

The problem seems the following method:

applyToShareJS = function(editorDoc, delta, doc) {
    var getStartOffsetPosition, pos, text;

    getStartOffsetPosition = function(range) {
      var i, line, lines, offset, _i, _len;

      lines = editorDoc.getLines(0, range.start.row);
      offset = 0;
      for (i = _i = 0, _len = lines.length; _i < _len; i = ++_i) {
        line = lines[i];
        offset += i < range.start.row ? line.length : range.start.column;
      }
      return offset + range.start.row;
    };
    pos = getStartOffsetPosition(delta.range);
    switch (delta.action) {
      case 'insertText':
        doc.insert(pos, delta.text);
        break;
      case 'removeText':
        doc.del(pos, delta.text.length);
        break;
      case 'insertLines':
        text = delta.lines.join('\n') + '\n';
        doc.insert(pos, text);
        break;
      case 'removeLines':
        text = delta.lines.join('\n') + '\n';
        doc.del(pos, text.length);
        break;
      default:
        throw new Error("unknown action: " + delta.action);
    }
  };

Where delta is defined different.

mizzao commented 8 years ago

Do you know what version this hit? Maybe we'll just stick to the pre 1.2.0 version for now.

DavidSichau commented 8 years ago

Version 1.1.9 should be OK. Should I make a Bug report at ShareJs?

mizzao commented 8 years ago

No, because that is ShareJS 0.6 and I don't think it's being maintained anymore. You could certainly write something for the 0.6 branch, but I doubt anyone will read it.

We should update to ShareJS 0.7 at some point, which could also allow things like rich text: #50

mizzao commented 8 years ago

Can you verify that 1.1.9 works and I can re-publish the ace package?

EDIT: I actually just published mizzao:sharejs-ace@1.1.9 (didn't know Meteor could publish old versions now) so give that a try.

mizzao commented 8 years ago

For anyone else reading this thread, the solution is to meteor add mizzao:sharejs-ace@1.1.9 for now. (note explicit version).

DavidSichau commented 8 years ago

Version 1.1.9 works fine. Thanks a lot.

rubencodes commented 8 years ago

As an EDIT to the above, to force 1.1.9 instead of 1.2.0, you must enter: meteor add mizzao:sharejs-ace@=1.1.9

edemaine commented 8 years ago

Agreed; the missing equal sign needs to be added to README.md

toncaa commented 8 years ago

Yea that should be changed. Without "=" sign, it installs newest version. So mizzao for greater good, update README.md.

edemaine commented 8 years ago

If newer Ace is now supported, should you remove "@1.1.9" from README.txt?