search

eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.84k stars 2.48k forks source link

Refactor auto save mechanism #13683

Closed msujew closed 3 months ago

msujew commented 3 months ago

What it does

Closes https://github.com/eclipse-theia/theia/issues/12863 Closes https://github.com/eclipse-theia/theia/issues/12844 Closes https://github.com/eclipse-theia/theia/issues/8722

Enhances the SaveResourceService to perform all the auto save preference work. Previously, all editor classes had to do their own handling of the afterDelay auto save functionality. This has been centralized into one service. Therefore, custom editors and notebook editors now behave exactly the same as monaco text editors in regards to auto saves.

This also fixes a long standing issue, where onFocusChange and onWindowChange have been handled as if afterDelay was selected as the auto save mode. These two values are now properly handled.

Note that onWindowChange doesn't work well with custom editors. This is due to the usage of iframe and the inability to look into an iframe. It still works, but is a bit too eager with them.

How to test

Basically, confirm that the auto save functionality behaves as expected with all different values of the enum preference. This should be tested for all 3 kinds of editors (text, notebook, custom editor).

Also test that closing unsaved editors prompts (or automatically saves) the user whether or not to save those files.

Follow-ups

We might want to find a way to fix onWindowChange for custom editors as well based on mouse position event information.

Review checklist

Reminder for reviewers

tsmaeder commented 3 months ago

I've found a weird little bug:

  1. Turn autosave to "off".
  2. Open an editor
  3. Types something
  4. Close the editor
  5. Observe: you get a dialog asking you to save the dirty editor

Works as expected.

  1. Open the settings, set set autosave to "after delay"
  2. Open an editor
  3. Type something in the editor
  4. Observe: after a delay, the "dirty" indicator in the editor tab disappears
  5. Open the "File" menu: there is a checked menu item called "Autosave". Select the item
  6. Open the "File" menu again: observe that the "Autosave" item is unchecked
  7. Type in the editor.
  8. Observe: the "dirty" indicator never goes away.
  9. Close the editor
  10. Observe: there is no dialog asking me to save and the changes I make are saved to disk automatically. My expectation is that I am asked to save the dirty editor upon closing.

Note that I can have the settings open and observe the "autosave" setting change to "off" when I uncheck the menu item, but I only get the "save"-dialog back once I manually edit the setting in the settings editor.

msujew commented 3 months ago

@tsmaeder Thanks for the repro steps. I believe I ran into a variation of https://github.com/eclipse-theia/theia/issues/7480 when using PreferenceChange#newValue, which was undefined after the auto save preference was updated. I'm now using the computed value instead, which seems to work well.

tsmaeder commented 3 months ago

@tsmaeder Thanks for the repro steps. I believe I ran into a variation of #7480 when using PreferenceChange#newValue, which was undefined after the auto save preference was updated. I'm now using the computed value instead, which seems to work well.

From the linked issue :

It's just a matter of time until someone thinks: "I'll just use the new value" and introduces an error.

😎 🤣