Implement #1452

[NotLazy]

Issue #, if available: #1452

Description of changes: Implements the toJSON and fromJSON methods to allow saving and restoring undoManager's state.

// Example
var undoManager = editor.session.getUndoManager();
var state = undoManager.toJSON();
// ... then later ...
undoManager.fromJSON(state);

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[NotLazy]

Honestly I'm disappointed this hasn't been done sooner, and that the undoManager has had empty toJSON and fromJSON methods for this long.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7a6b83e) 87.41% compared to head (73b7c02) 87.43%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5236      +/-   ##
==========================================
+ Coverage   87.41%   87.43%   +0.01%     
==========================================
  Files         574      574              
  Lines       45613    45652      +39     
  Branches     6935     6935              
==========================================
+ Hits        39873    39914      +41     
+ Misses       5740     5738       -2     

Flag	Coverage Δ
unittests	87.43% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/undomanager.js	78.39% <100.00%> (+0.89%)	⬆️
src/undomanager_test.js	99.71% <100.00%> (+0.03%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andredcoliveira]

Hey! Thank you for the contribution! Empty toJSON and fromJSON methods are definitely not useful.

The changes look good to me; could you add some basic tests that ensure the output is as expected?

[andredcoliveira]

Could you introduce testing for the two methods?

[phiden1]

Close

[NotLazy]

Close

This affects undomanager not edit session. Don't close.

[NotLazy]

Hey! Thank you for the contribution! Empty toJSON and fromJSON methods are definitely not useful.

The changes look good to me; could you add some basic tests that ensure the output is as expected?

I'll add tests as soon as I have a chance.

[NotLazy]

Small little update about the usage of these changes, if you want to save state to localStorage or upload to remote server.

// Example
var undoManager = editor.session.getUndoManager();
var state = undoManager.toJSON();
var json = JSON.stringify(state);
// ... then later ...
undoManager.fromJSON(state);

That is to say, the toJSON method actually returns a "safe" instance object, not a string.

Just let me know if you'd prefer a string and I'll happily make the change.

[NotLazy]

@andredcoliveira This is ready to merge now, unless you'd rather me convert the methods to return/accept a string instead of an object.

[InspiredGuy]

Thank you for the contribution! Returning object is fine, it is consistent with existing toJSON method in Ace codebase.