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

OT: Revise deltas transformations #4099

Closed
scofalik opened this issue Jul 5, 2017 · 2 comments
Closed

OT: Revise deltas transformations #4099

scofalik opened this issue Jul 5, 2017 · 2 comments
Labels
package:engine resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:bug This issue reports a buggy (incorrect) behavior.

Comments

@scofalik
Copy link
Contributor

scofalik commented Jul 5, 2017

(Follow-up to ckeditor/ckeditor5-engine#977)

ckeditor/ckeditor5-engine#977 Changes a lot in operations transformation algorithms and delta transformation algorithms. Deltas priority was removed. Additionally, more edge cases pops up, since deltas are no longer deleted from history on undo. Also, we generally gained a lot of experience since special cases for deltas transformations were written.

This all means that delta transformation cases should be revised, all of them, and check whether they are actually correct. In ckeditor/ckeditor5-engine#977 I already re-written SplitDelta x SplitDelta and refactored SplitDelta x RemoveDelta transformations. I am sure that there are errors in other cases (although no tests fail ATM).

@scofalik
Copy link
Contributor Author

scofalik commented Aug 1, 2017

Three things to take care of when doing this issue:

  1. Do we need special cases for InsertDelta x MergeDelta and MoveDelta x MergeDelta transformations? From what I've written down we should be fine with default transformations but I haven't spend enough time on this. Undo and OT need to be taken into consideration.

  2. We probably need InsertDelta x SplitDelta special case, cause inserted node may end up between split nodes. It's not the end of the world, but it may happen.

  3. At the moment of writing this comment, a lot of special cases either are already skipped in "undo mode" or looks like skipping would be at least "profitable". It was like this because some delta transformations caused bugs when delta was first transformed by another delta and then by a delta which reversed it. Skipping in "undo mode" is easy but dangerous. What will happen if a delta that is essentially a "reversed delta" will be applied outside of undo? Easy example is pressing enter and then backspace, it is same as enter and undo but the former won't be seen as "undo mode".

TBH, I'd rather have less special cases but only those that are undoubtedly needed.

@scofalik
Copy link
Contributor Author

Since deltas will be changed to operations, this issue is invalid.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added module:model resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:bug This issue reports a buggy (incorrect) behavior. package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

2 participants