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

Delta: enable history log when delta becomes transformed #4061

Closed
scofalik opened this issue May 8, 2017 · 2 comments · Fixed by ckeditor/ckeditor5-engine#941
Closed

Delta: enable history log when delta becomes transformed #4061

scofalik opened this issue May 8, 2017 · 2 comments · Fixed by ckeditor/ckeditor5-engine#941
Assignees
Labels
package:engine type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@scofalik
Copy link
Contributor

scofalik commented May 8, 2017

It would make debugging easier, if delta instance kept a history of all deltas by which it was transformed, with all additional information that is needed to recreate that delta from it's original state to current state.

This is important to debug OT.

@scofalik scofalik self-assigned this May 8, 2017
@scofalik
Copy link
Contributor Author

scofalik commented May 8, 2017

Delta#history is created and filled automatically when engine/model/delta/transform~deltaTransform#transform is used, if engine debugging is on (engine/dev-utils/enableenginedebug~enableEngineDebug was called).

Delta#history is an array. Each item is an object containing properties:

  • {String} before - JSON string containing delta state before transformation,
  • {String} transformedBy - JSON string containing delta which before was transformed by,
  • {Boolean} wasImportant - whether before was more important than transformedBy when it was transformed,
  • {Number} resultsTotal - how many deltas were returned as the transformation result,
  • {Number} resultIndex - what was the index of that delta in the transformation result.

This info will let us recreate all steps which led to current delta state, even if it was transformed multiple times. First item's before property is the original delta. Then it has to be transformed by transformBy deltas of following items (using wasImportant). After each transformation, resultIndex delta has to be chosen from among returned deltas.

Non-first before values and resultsTotal values can be used for additional checking or when there is no need to perform transformation back from the original delta. Those could be omitted/removed if we don't want them.

Note: before and transformedBy JSON strings do not contain their own history properties. That would lead to enormous data duplication.

@scofalik
Copy link
Contributor Author

scofalik commented May 8, 2017

I did some changes to how functions are exported in delta/transform.js. Fortunately, there are only two repositories which uses those - engine and undo (however in undo it should be changed anyway).

I found those changes the only sane way to enable this feature without transformation algorithms directly "doing something" when "debugging is enabled". Unfortunately this is the only way to substitute/overwrite functions and it probably is not possible if transform.js methods are exported as singular functions.

scofalik referenced this issue in ckeditor/ckeditor5-engine May 9, 2017
Feature: When engine debugging is on, deltas that are results of transformation will keep their history of changes in `#history` property. Closes #940.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added type:improvement This issue reports a possible enhancement of an existing feature. 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 type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants