Skip to content

adds setText to text api.#88

Open
CrypticSwarm wants to merge 1 commit into
josephg:masterfrom
CrypticSwarm:textapi-add-settext
Open

adds setText to text api.#88
CrypticSwarm wants to merge 1 commit into
josephg:masterfrom
CrypticSwarm:textapi-add-settext

Conversation

@CrypticSwarm

Copy link
Copy Markdown
Contributor

Closes issue #35
(#35)

Uses existing del and insert ops.

Closes issue josephg#35
(josephg#35)

Uses existing del and insert ops.
@josephg

josephg commented May 5, 2012

Copy link
Copy Markdown
Owner

Ok to merge, @nornagon ?

@nornagon

nornagon commented May 5, 2012

Copy link
Copy Markdown
Contributor

I'm happy with @CrypticSwarm's reasoning :)

@wmertens

wmertens commented Aug 1, 2012

Copy link
Copy Markdown
Contributor

looks like this cannot be merged automatically any more?

@CrypticSwarm

Copy link
Copy Markdown
Contributor Author

Most likely due to the generated compressed code. I should probably not have included those in the PR.

The fix would be to rebuild and accept the rebuilt version.

@wmertens

wmertens commented Aug 1, 2012

Copy link
Copy Markdown
Contributor

Well, it still needs the unit test that Joseph wants too :-)

@wmertens

Copy link
Copy Markdown
Contributor

I gave it a try with a test, and I had to add the same function to all the text APIs. That works for most but not for text-tp2.

Take a look at wmertens@6e84dc7

Any way to implement it better for text-tp2?

@wmertens

Copy link
Copy Markdown
Contributor

@CrypticSwarm ping?

@CrypticSwarm

Copy link
Copy Markdown
Contributor Author

Eeep. Not sure off hand. It is probably related to how tp2 works. I think tp2 leaves tombstones in place of deletes instead of deleting them. Perhaps that is making it fail.

@wmertens

wmertens commented Mar 1, 2013

Copy link
Copy Markdown
Contributor

Escalating to @josephg :-) What do you think Joseph?

On Feb 28, 2013, at 21:08 , CrypticSwarm notifications@github.com wrote:

Eeep. Not sure off hand. It is probably related to how tp2 works. I think tp2 leaves tombstones in place of deletes instead of deleting them. Perhaps that is making it fail.


Reply to this email directly or view it on GitHub.

@josephg

josephg commented Apr 14, 2013

Copy link
Copy Markdown
Owner

This is a really roundabout way to replace the text contents, and it still has the bug @nornagon was talking about --- if the first submit fails, the error is discarded.

Why not just submit a single operation that deletes the entire document and inserts the string you want?

For the non composable text type, make an op that deletes everything then inserts text:

  setText: (text, callback) ->
    op = [{p:0, d:@snapshot.length}, {p:0, i:text}]
    @submitOp op, callback
    op

For composable text, do the same thing:

  setText: (text, callback) ->
    op = [{d:@snapshot.length}, text]
    @submitOp op, callback
    op

For TP2 its a little more complicated, because you need to delete all the characters, which have tombstones between them. Luckily, deleting a tombstone is a no-op so we can just delete everything.

  setText: (text, callback) ->
    op = [{d:@snapshot.totalLength}, {i:text}]
    @submitOp op, callback
    op

Interestingly, in the TP2 case you could swap the insert & delete and submit [{i:text}, {d:@snapshot.totalLength}] instead. The difference is that in the first case, everyone's cursors would teleport to the start of the document and in the second case they'd all float to the end.

There's a comment at the top of each OT type that tells you what operations look like. Take a look at the comment describing the TP2 type to understand a bit of whats going on there.

@josephg

josephg commented Apr 14, 2013

Copy link
Copy Markdown
Owner

Also, this is missing tests.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants