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

Updating/Insetring of wrong nodes fixed. Closes #104 #106

Merged
merged 4 commits into from
Oct 28, 2015

Conversation

Kanaye
Copy link
Collaborator

@Kanaye Kanaye commented Oct 24, 2015

For details read the message of the first commit 😉.
If you find an edge case I didn't fix or you don't like something about this solution feel free to say.
Closes #104

Joscha Rohmann added 2 commits October 25, 2015 00:06
Added an rendering type Expression.NodeWise that will return an array with
the values positioned in the order of the node when passed to
Expression.GetValue.

Expressios objects now have a property nodeLength and chunks of
expressions now have an array nodePositions containing the the position of
the nodes the expression updates.

Updated VirtualElement.syncChildren to delete all nodes related to an
expression and to update all nodes of an expression with a changed value.
Added tests for updateing and inserting/deleting the right nodes
"VirtualElement.syncChildren()"
@astoilkov
Copy link
Owner

I will take a look at the solution for this soon and will merge it.

@Kanaye
Copy link
Collaborator Author

Kanaye commented Oct 26, 2015

I removed the [chunk of expression].nodePositions as it was originaly for an other way to implement this behavior and is unnecessary for this implementation and just blew up the code.

Also just take your time to review the changes. The PR-might change over time (as it did now) but it won't run away 😄.

Also edited test so that it catches bugs like that.
@astoilkov
Copy link
Owner

Wow. Cool. We have a lot of more because we are launching Caret. I will take a look in the next couple of days. This way you will also have time to make changes if you want. :)

@astoilkov
Copy link
Owner

I am ready to merge the pull request. Do you have any additional changes you want to add before that?

@Kanaye
Copy link
Collaborator Author

Kanaye commented Oct 28, 2015

Great! No I have no additional changes.

astoilkov added a commit that referenced this pull request Oct 28, 2015
Updating/Insetring of wrong nodes fixed. Closes #104
@astoilkov astoilkov merged commit 90163bd into astoilkov:master Oct 28, 2015
@Kanaye Kanaye deleted the VE-sync-ffixes branch October 28, 2015 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocks is inserting the wrong text in the wrong node
2 participants