Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ace editor 1.2.0 is compatible with ShareJS 0.6 #55

Closed
DavidSichau opened this issue Sep 29, 2015 · 10 comments · Fixed by #70
Closed

Ace editor 1.2.0 is compatible with ShareJS 0.6 #55

DavidSichau opened this issue Sep 29, 2015 · 10 comments · Fixed by #70

Comments

@DavidSichau
Copy link
Collaborator

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

ajaxorg/ace#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
Copy link
Owner

mizzao commented Sep 29, 2015

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

@DavidSichau
Copy link
Collaborator Author

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

@mizzao
Copy link
Owner

mizzao commented Sep 29, 2015

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
Copy link
Owner

mizzao commented Sep 29, 2015

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 mizzao changed the title Ace 1.2 not compatible to sharejs ace Ace editor 1.2.0 is compatible with ShareJS 0.6 Sep 29, 2015
@mizzao
Copy link
Owner

mizzao commented Sep 29, 2015

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

@DavidSichau
Copy link
Collaborator Author

Version 1.1.9 works fine.
Thanks a lot.

@rubencodes
Copy link

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
Copy link

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

@toncaa
Copy link

toncaa commented Jan 22, 2016

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

@edemaine
Copy link

edemaine commented May 8, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants