eclipse-emfcloud / theia-tree-editor

theia-tree-editor
Other
15 stars 12 forks source link

adapt to latest theia 1.24 #57

Closed alxflam closed 2 years ago

alxflam commented 2 years ago

Hi,

with the current Theia 1.24 (and already with 1.23.0) the tree editor cannot be saved anymore. See changes in Theia#10736 and https://github.com/alxflam/theia/commit/e62fd33ce46055d1633d781c08bf5852535f53c5. Monaco now defines different save preferences (the keys as well as the allowed values have changed).

Changes:

I had to add a "eslint-disable-next-line import/no-deprecated" to the import of createTreeContainer as there are now two such functions, whose only difference is the type of an optional parameter (and one of these functions is deprecated). Don't know how i can trick typescript to import the not-deprecated function if they are actually named identically.

I did not implement 'onFocusChange' / 'onWindowChange', instead i kept it simple and always save with a delay (delay from the preferences). We may need to look into this once more.

Greetings, Alex

alxflam commented 2 years ago

@lucas-koehler thanks for your feedback, i'll try to summarize my observations:

Now BaseTreeEditorWidget only holds the current state in the instanceData field, but it cannot load the currently persisted state again (would be required for revert). But createSnapshot could simply return the current instanceData. The same is true for NavigatableTreeEditorWidget.

ResourceTreeEditorWidget instead could implement revert by calling load to restore the currently persisted state of the resource and in createSnapshot return this.data.

A few remarks:

For now, i have added a default implementation for revert to ResourceTreeEditorWidget and createSnapshot for BaseTreeEditorWidget. I think the revert for now needs to be handled by the implementing subclass (if it's not a ResourceTreeEditorWidget). Additionally, ResourceTreeEditorWidget now also reuses the parents instanceData member (else i could not reuse the parents createSnapshot implementation).

alxflam commented 2 years ago

@lucas-koehler thanks for testing, i missed out checking the examples. Now the example should work. I've implemented applySnapshot for the ResourceTreeEditorWidget (notice that setTreeData needs to be called to ensure the view for the newly saved file is also updated and shows the applied state). The object type used for createSnapshot and applySnapshotis the same one the MonacoEditorModel uses (as we need some kind of object to wrap the primitive string).

lucas-koehler commented 2 years ago

@alxflam Thanks for the update and the contribution in general :). The error message no longer appears :) The CI currently fails but that does not seem to be related to your changes (it cannot download a dependency). I'll rerun it again in a few hours to see if it works better again :D