Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Introduced relations in OT #1478

Closed
wants to merge 5 commits into from
Closed

Introduced relations in OT #1478

wants to merge 5 commits into from

Conversation

scofalik
Copy link
Contributor

@scofalik scofalik commented Jul 27, 2018

Suggested merge commit message (convention)

Fix: Introduced relations in OT which helps solving conflicts during undo. Closes ckeditor/ckeditor5#4383.

@scofalik
Copy link
Contributor Author

This PR reduces the number of skipped tests to 4, of which only 2 are in transformations (the other 2 are related to a different part of the changed code - engine debug tools).

@coveralls
Copy link

coveralls commented Jul 27, 2018

Coverage Status

Coverage decreased (-0.4%) to 98.409% when pulling 647971b on t/1468 into 00fbf7f on master.

@scofalik
Copy link
Contributor Author

After some changes, it requires #1486

@@ -53,6 +53,12 @@ describe( 'transform', () => {
}
}

const contextIsStrong = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the name suggests a boolean option, not an object.

}

const origB = this.originalOperations.get( opB );
const undoneB = this.document.history.getUndoneOperation( origB );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undone operation? Comment is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what exactly should be commented here, but I'll add extensive comments in a followup to explain every of those algorithms.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, it was late. ;)

@pjasiun
Copy link

pjasiun commented Aug 9, 2018

Moved to #1497.

@pjasiun pjasiun closed this Aug 9, 2018
@pjasiun pjasiun deleted the t/1468 branch August 9, 2018 12:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants