added some fixes from @tarr11 to codemirror integration #119

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

added suggested fixes from @tarr11: https://github.com/tarr11/codeshare/

the old webclient/cm.js didn't works properly. this one is working the same way the ace backend (at least in my tests).

I also changed examples/cm/index.html. so we can test this on the bin/exampleserver and then http://localhost:8000/cm

fix-cm branch.

thanks about your incredible work on sharejs!

This pull request passes (merged cf7cc46 into c366f99).

Is there a reason you are no longer handling errors? Not that the old code did a really great job of it, but this is example code and people will base their code on it...

Owner

automata replied Aug 20, 2012

I'm using it on this app and it is working well for now. I got this tips from @tarr11. I'm going to check better and return. Thanks!

Contributor

wmertens commented Aug 14, 2012

One more thing, you just recompiled the webclient right (cake webclient)? Or did you make changes to it? The webclient is compiled from src/client/ and any changes you made should be made there...

Contributor

wmertens commented Oct 23, 2012

@automata So can you add the changes you made to https://github.com/josephg/ShareJS/blob/master/src/client/cm.coffee ? This pull request only has cm.js which gets overwritten every time someone runs cake webclient

@hatpick hatpick commented on the diff Nov 27, 2012

webclient/cm.js
return check();
});
- this.detach_cm = function() {
- editor.setOption('onChange', null);
- return delete this.detach_cm;
+ doc.detach_codemirror = function () {
+ editorDoc.removeListener('change', editorListener);
@hatpick

hatpick Nov 27, 2012

Contributor

This should be editorDoc.setOption('onChange', null) for codemirror 2+

Contributor

wmertens commented Jan 18, 2013

@automata I would love to merge this but in its current state I can't. Any chance you could change the pull request like discussed in the comments?

Thanks!

Contributor

wmertens commented Feb 28, 2013

@automata ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment