Show remote carets #23
Conversation
|
Just realized that it works well when there are only two peers, but fails with more than that. The problem is that upon recieving the change event, the So in scenario:
So, each peer should have probably his own |
main.js
Outdated
| newEditor._codeMirror.on('change', sendLocalChange) | ||
| newEditor._codeMirror.on('beforeSelectionChange', sendLocalSelect) | ||
| newEditor._codeMirror.on('change', onCodeChange) | ||
| newEditor._codeMirror.on('changes', onCodeChanges) |
There was a problem hiding this comment.
If we are going to start listening to operation-batched changes, we should only use the 'changes' event.
| return range.head.ch !== range.anchor.ch || range.head.line !== range.anchor.line | ||
| } | ||
|
|
||
| function sendLocalCaretsMoves (cm, change) { |
There was a problem hiding this comment.
Do you mind adding a few comments to this to explain how you are calculating the caret positions from the changes?
There was a problem hiding this comment.
Sure, I'll sit to it when finish your review :)
main.js
Outdated
| @@ -396,13 +450,30 @@ define(function (require, exports, module) { | |||
| }) | |||
| } | |||
|
|
|||
| function handleRemoteCarets (data) { | |||
| if (fromWebPath(data.filePath) !== documentRelativePath) return | |||
There was a problem hiding this comment.
Fine for now, but add a TODO so we can support tracking carets in closed files. (Needs to be done for selections to if I remember correctly.)
|
Looks good, thanks for figuring this out. I added a few comments. |
main.js
Outdated
| @@ -34,6 +34,7 @@ define(function (require, exports, module) { | |||
| var nickname = null | |||
| var knownDocs = {} | |||
| var room | |||
| var remoteCarets = new Set() | |||
There was a problem hiding this comment.
I don't use ES6 anywhere else. Please use an object set and use keys to iterate.
|
Guess that's it. Besides the requested changes I've left a comment about the issue I mentioned in #23 (comment) earlier. |
|
OK, seems to be working great aside from the issue with >2 users. I'll fix that in a later commit when I do cursor colours. Thanks @Worie ! Would you like to be added as a maintainer? I can always use some more help developing this :) |
|
I'm glad it's ok :) When it comes to me as a maintainer - I can't really promise anything as I don't know how much spare time will I have, but sure - that'd be nice, thanks! |
Here we go. I tried to follow your codestyle, I guess it should be ok. The changes provided should cover every input from peers (multiple carets, removals, additions, pastes and cuts)
If this PR's alright, then in the future I guess we should create a html template for carets (for future features such as displaying the carets owner name and for code maintenance purposes)
This solves #22