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

T/undo 2 #14

Merged
merged 16 commits into from
Jun 24, 2016
Merged

T/undo 2 #14

merged 16 commits into from
Jun 24, 2016

Conversation

scofalik
Copy link
Contributor

@scofalik scofalik commented Jun 22, 2016

This PR includes a rewrite of Undo feature, based on experience gained before/during/after hackathon when I had a chance to play a lot with undo and come up with multiple weird scenarios. It fixes ckeditor/ckeditor5#2684 ckeditor/ckeditor5#2689 ckeditor/ckeditor5#2685 ckeditor/ckeditor5#2690. Funny enough, ckeditor/ckeditor5#2690 wasn't only optimization thing but, when applied to operational transformations done by undo, also enabled creating undo algorithm that finally outputs correct results.

Other than that the PR introduces:

  • Extracting redo stuff from RedoCommand as the redo algorithms divereged heavily from undo (it was one command before)
  • BaseCommand which is a base for UndoCommand and RedoCommand

Needs ckeditor/ckeditor5-engine#499

@pjasiun pjasiun self-assigned this Jun 22, 2016
@scofalik scofalik force-pushed the t/undo-2 branch 3 times, most recently from 7388779 to c9e8248 Compare June 23, 2016 09:50

// Performs a transformation of delta set `setToTransform` by given delta set `setToTransformBy`.
// If `setToTransform` deltas are more important than `setToTransformBy` deltas, `isStrong` should be true.
export function transformDelta( setToTransform, setToTransformBy, isStrong = false ) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it part of the public API?

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 think we can imagine some clearing mechanisms in model that will like to clear undo stack. We were discussing things like setData recently. If needed, this method can be expanded to clear only first X items, etc.

* reversed delta that was later transformed.
*
* But what about that selection? For batch `C`, undo feature remembers selection just before `C1` was applied. It can be
* visualized between delta `B2` and `B3` (see fig. 3). Obviously a lot happened to the document since the selection
Copy link

Choose a reason for hiding this comment

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

"a lot"? Isn't it only "B3" since we reversed "C"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe "a lot" is overstating here, yes. But what I've meant is that we actually applied five deltas. But it's true that for that selection only B3 happens. I'll change it.

@pjasiun pjasiun merged commit 0a53018 into master Jun 24, 2016
@pjasiun pjasiun deleted the t/undo-2 branch June 24, 2016 12:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undo command should enqueue its changes
3 participants