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

Hot exit on switching folders #15467

Closed
Tyriar opened this issue Nov 14, 2016 · 30 comments · Fixed by #18626
Closed

Hot exit on switching folders #15467

Tyriar opened this issue Nov 14, 2016 · 30 comments · Fixed by #18626
Assignees
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded workbench-hot-exit Preservation of unsaved changes across restarts
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Nov 14, 2016

Parent issue: #101

Sublime has a feature that allows hot exit to always be allowed on a window provided it has a project associated with it. The initial concern still remains that backups may go missing if we just always enable this, the Sublime project system is nice because the act of creating a project (.sublime-project) is basically an opt in for project-level features.

Interested in people's ideas on this, here are a few:

  • Enabling switching folders all the time?
  • Enable if <project>/.vscode exists?
  • Enable if a setting is toggled?
  • Enable if a setting includes the workspace path? (has issues with multi-folder workspaces)
@Tyriar Tyriar added feature-request Request for new features or functionality workbench-hot-exit Preservation of unsaved changes across restarts labels Nov 14, 2016
@Tyriar Tyriar added this to the Backlog milestone Nov 14, 2016
@Tyriar Tyriar self-assigned this Nov 14, 2016
@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

/cc @bpasero

@bpasero
Copy link
Member

bpasero commented Nov 14, 2016

@Tyriar can you elaborate on the issue? you say I loose backups if I switch folders with dirty files opened?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

In Sublime you can do this:

  1. Open folder 1
  2. Open folder 2
  3. Create a .sublime-project for folder 2
  4. Dirty folder 2 workspace (creates .sublime-workspace as a sibling to the .sublime-project)
  5. Close folder 2 (it's allowed even though it's not the last window because it's a "project")

These backups are not restored if Sublime is then fully exited and relaunched.

We could fairly easily change VS Code to allow this by default, but the concern is that Untitled backups for example could go missing by users accidentally closing the non-last window and not getting it restored.

How #396 is solved could impact this as we may need to introduce a project concept.

@bpasero
Copy link
Member

bpasero commented Nov 14, 2016

@Tyriar but is that 2 folders in the same window or in multiple windows? Do you imply that Sublime enables hot exit in this case even though the user did not enable it?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

@bpasero folder 1 and folder 2 in the above example are in different windows. Hot exit is enabled by default, I'm assuming this is disabled as well when it's disabled.

@bpasero
Copy link
Member

bpasero commented Nov 14, 2016

Ah so today for us when you close window 2 we ask for saving even though hot exit is enabled? That is the point I was missing.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

Yeah that's right. I suggest this be deferred until after #396 is figured out.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 19, 2016

Looked at Notepad++, it appears to have the same restrictions as Sublime Text where you can only close the last window. It also has a workspaces concept, however I did the following:

  1. Opened a second window
  2. Opened a folder as a workspace
  3. Dirtied some files
  4. Closed it, it exited without prompt

But then I couldn't figure out how to restore the old files and hot exit seems to not work at all any more.

Tyriar added a commit to microsoft/vscode-docs that referenced this issue Dec 14, 2016
This section be expanded upon when microsoft/vscode#15467 is resolved
@egamma egamma mentioned this issue Jan 4, 2017
56 tasks
@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2017

Proposal

I'm proposing the following to allow hot exit to be more flexible on what types of exits trigger a hot exit and also what exactly is restored on initial launch.

Settings

{
  // Whether hot exit is enabled which allows changes to unsaved files to be
  // remembered between sessions. Setting this to false will override all other
  // files.hotExit settings.
  "files.hotExit.enabled":
      true (default) | false,
  
  // What windows with backups should be restored when VS Code is launched (on
  // Mac this requires the application to be quit completely).
  "files.hotExit.restoreOnStartUp":
      "none" | "nonFolderWindows" | "folderWindows" | "all" (default),
  
  // What types of workspaces trigger a hot exit when a single window is
  // closed. Note that if VS Code is exited completely via the
  // workbench.action.quit command (via command pallete, keybinding of menu),
  // a hot exit will always trigger.
  "files.hotExit.backupOnWindowClose":
      "all" | "nonFolderWindows" | "folderWindows" | "none" (default),
}

The settings are hopefully self-explanatory, this had a number of iterations before posting this and I landed on this instead of a few pre-determined configurations as I believe this is a very personal decision since multiple people I asked wanted different things. Also with the above there is no change in the default behavior (conservative).

The settings themselves also look much less intimidating when there is only the default value, so power users who want to configure to their desire are free to do so:

{
  "files.hotExit.enabled": true,
  "files.hotExit.restoreOnStartUp": "all",
  "files.hotExit.backupOnWindowClose": "none",
}

This is my personal preference for example:

{
  "files.hotExit.enabled": true,
  
  // If I explicitly close a folder window I'll restore it when I reopen the folder.
  "files.hotExit.restoreOnStartUp": "nonFolderWindows",
  
  // I'd prefer not to accidentally close non-folder windows but clean up after them.
  "files.hotExit.backupOnWindowClose": "folderWindows",
}

This will probably be a common configuration:

{
  "files.hotExit.enabled": true, // (default)
  "files.hotExit.restoreOnStartUp": "all", // (default)
  "files.hotExit.backupOnWindowClose": "all",
}

Menu/command to restore

Since not everything will potentially be restored on launch anymore, a menu and/or command is necessary to surface the currently backed up windows to the user. This is what I'm thinking for the menu item:

File > Restore Backed Up Window
  Restore all
  ---
  Non-folder window #1
  Non-folder window #2
  ~/dev/Microsoft/vscode
  ~/dev/Microsoft/vscode

The sub menu looks very similar to the Open Recent menu only with no clear (it's too destructive to have in the menu) and it features "Non-folder window #n" which I can't think of a better name for just yet.

folderWindows vs nonFolderWindows

Currently we name VS Code windows with no folder "empty workspaces" and those with a folder open "workspaces", this is confusing to many people and might be even more difficult to get across when multiple root workspaces #396 comes in. In addition it's been suggested we'll move towards identifying windows as windows rather than the workspaces they contain to make multiple windows with the same root #2686. So it makes sense to move the terminology towards something more understandable like this (or similar if there is a better alternative).

Other considerations

  • We may want to limit the number of backups and/or restores if we allow random window closes to hot exit but all windows to be restored in order to prevent a situation where the PC can't handle launching so many instances at once.
  • Since it's very easy to quit accidentally now (cmd+q/ctrl+q) with no dialogs and things may not restore in their previous state (unless "windows.reopenFolders": "all"), it might be a good idea to introduce a window.promptOnApplicationExit boolean setting which shows a dialog before application exit.

@bpasero
Copy link
Member

bpasero commented Jan 6, 2017

Wow, I am reading through this and even though I know exactly how hot exit works because I reviewed the code and contributed to it I am having a hard time understanding either the need for the settings or the need for a special menu item to restore backup windows. I am also not seeing these options in other applications that support hot exit. I wonder if here we are going a bit too far with what you can configure and should rather just settle on a default that we think is right and leave it like that...

Before going much into technical detail of what you want to change, what are the use cases for these changes?

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2017

The main difference is that other editors that support backing up folder not on application exit (np++, sublime) have our conservative default, but will allow it when there is a "workspace file". We don't have a concept of like workspace files that live outside the folder, we have workspace settings but they are inside the folder and won't work for multiple root repositories.

The restoreOnStartup setting would enable the user to decide whether they want all backups to restore on launch or a subset depending on window type. The former here would potentially flood the screen with windows which is a big no-no as we're built on electron. Consider the user that keeps vscode open for a week but closes many dirty windows off in the time (easy to do on a Mac), in this case the user could get into a situation where launching vscode creates 50 windows and they fill RAM on launch.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2017

This issue is the primary use case we want to enable, where folder windows can be closed and they will not necessarily be restored on launch.

@bpasero
Copy link
Member

bpasero commented Jan 6, 2017

@Tyriar would it be so bad to just restore every folder that has unsaved files on the next full restart? and since we restore backups also when the folder is opened again, a user actually has a way to get back to the unsaved files by just opening them. why do we have to put the workspaces with unsaved files so prominently into the File menu?

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2017

That's just a suggestion in order to make it easier for the user to know what backups VS Code is tracking, it could just as easily be a quick pick via the command palette but that is much harder to discover. The 2 settings are the main things parts of the proposal, but since restoreOnStartup could be not "all" then I would want some way of telling the user what's backed up.

@bpasero
Copy link
Member

bpasero commented Jan 6, 2017

Maybe we should first look into our future of supporting multiple folders in one window and multiple windows on the same folder and thus potentially a similar concept as the workspace file and then decide what it means for hot exit? I am worried that we introduce lots of options and settings that might become obsolete again.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 6, 2017

The options are better in pretty much every way to saving only when a folder(s) are saved as "workspaces" imo. This way users can define whether they just want it to work on all window closes (opt-in) or none (safe default), rather than needing to explicitly save a workspace file (which we wouldn't want to introduce for single folders anyway).

@bpasero
Copy link
Member

bpasero commented Jan 6, 2017

OK, well I think I expressed my concerns :)

@chrmarti
Copy link
Collaborator

chrmarti commented Jan 9, 2017

Another "keep it simple" vote:

  • Closing a window should always behave the same, independently of whether there are other windows open or not. ("files.hotExit.backupOnWindowClose": "all", without that setting)
  • Whether or not a window with backups should be restored, should depend on whether the user closed that window explicitly or the window/app crashed. If the user closed the window explicitly (close or quit), it was to get rid of the window for now and it should not be reopened. If the window was closed as part of a crash, the user is not necessarily aware of the dirty files and the window should be restored. ("window.reopenFolders": "all" would override that behavior, no need for the "files.hotExit.restoreOnStartUp" setting)

This is assuming that "files.hotExit.enabled" will stay to turn on/off the non-crash case and crashes will always benefit from the feature.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 9, 2017

@chrmarti Backups are restored regardless of whether files.hotExit.enabled is enabled/disabled.

Are you saying you think hot exit should just happen all the time for window closes as well? This would lead to potentially many backed up workspaces that would be restored on next launch. Consider the following scenario on a Mac:

  1. User opens VS Code
  2. Over the next week they open and close many different folders and empty workspaces, some dirty and some not
  3. They need to restart their Mac for some reason
  4. When VS Code is relaunched, all the backups will be restored (this could quickly become too many for the computer to handle)

The reason for the distinction between app exit and window close is if we only hot exit on app exit that means that all backups are visible all the time, reducing the chance backups would go missing or presenting backups from a long time ago (the exception to this being window crashes).

@chrmarti
Copy link
Collaborator

chrmarti commented Jan 9, 2017

@Tyriar I suggest to only restore windows as configured with the existing "window.reopenFolders" (including non-folder windows), except if there was a crash: then I would restore all windows open at the time of the crash. That seems to be simple and intuitive.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 9, 2017

I'm hesitant to remove the safe default as some users would expect the dialog and may think changes are already saved.

Here's another suggestion in between the current and your suggestion:

{
  // Whether hot exit is enabled which allows changes to unsaved files to be
  // remembered between sessions. Selecting "appExit" means that hot exit
  // will only be triggered when the application is closed
  // (workbench.action.quit command via command pallete, keybinding of menu),
  // all windows will be restored upon next launch. Selecting "appExitAndWindowClose" will
  // trigger hot exit when any folder window is closed, however only non-folder windows
  // will be restored when the application is restarted (not folder workspaces).
  "files.hotExit":
      "off", "appExit" (default), "appExitAndWindowClose"
}

Here appExitOnly implies these:

{
"files.hotExit.restoreOnStartUp": "all",
"files.hotExit.backupOnWindowClose": "none"
}

And "always" implies these:

{
"files.hotExit.restoreOnStartUp": "nonFolderWindows",
"files.hotExit.backupOnWindowClose": "folderWindows"
}

@Tyriar
Copy link
Member Author

Tyriar commented Jan 9, 2017

Another case for why the default should stay as is, consider a Mac user who has some unsaved work in a non-folder window and they close the window, expecting a save dialog. The dialog does not appear and they start to freak out, they try:

  • Closing their other windows
  • Opening new windows

Their work will not restore until they either kill the application completely via cmd+q or restart the computer.

@chrmarti
Copy link
Collaborator

chrmarti commented Jan 9, 2017

I like the single setting that offers the three most predictable behaviors. (After talking to you in person.)

@Tyriar
Copy link
Member Author

Tyriar commented Jan 15, 2017

@bpasero @chrmarti let me know what you think of this approach, using the single setting as proposed in #15467 (comment). It's working fine from what I can see but it would need a little more testing (plus fixing tests) 1af3873

Note that https://code.visualstudio.com/docs/editor/codebasics#_hot-exit would also go into more detail on what appExit and appExitAndWindowClose do and mention the synergy with "window.reopenFolders": "all".

My main concern with this approach is trying to briefly and succinctly differentiate the behaviors in the settings and the setting key itself only explains when the hot exit happens, not the change in restore behavior.

@bpasero
Copy link
Member

bpasero commented Jan 16, 2017

@Tyriar can you explain to me this limitation:

however only non-folder windows will be restored when the application is restarted (not folder workspaces).

As for the behaviour, when I have appExitAndWindowClose enabled, does this mean when I open another folder in the same window with dirty files, hot exit kicks in and I am not asked to save? Or does that only work when the window I am in is an empty workspace window (given that limitation warning)?

+1 for having this as enum inside the hot exit setting. My only concern is that maybe it should follow our files.autoSave naming and get rid of the obvious app in the name, so it would be onExit and on onExitAndWindowClose

@Tyriar
Copy link
Member Author

Tyriar commented Jan 16, 2017

That limitation is so people can close folders whrb they're done and open them again when they resume work on the folder. It's essentially this feature, #15467, so it acts more like sublime. The reason all non-folder workspaces are restored is to avoid needing a file menu by maintaining the "backups always visible" property for them.

I like the idea of dropped app 👍

@bpasero
Copy link
Member

bpasero commented Jan 16, 2017

@Tyriar maybe best to see with an example. Let's assume onExitAndWindowClose is set which as far as I understand is the new behaviour we are going to add here.

Can you fill in the blanks what happens in the following cases, assuming each have dirty files:

  • user closes the window of a folder workspace
  • user closes the window of a non-folder workspace
  • user clicks on File > Close Folder in a folder workspace
  • user selects File > Open Folder in a window of a folder workspace
  • user selects File > Open Folder in a window of a non-folder workspace

@Tyriar
Copy link
Member Author

Tyriar commented Jan 16, 2017

For onExitAndWindowClose:

user closes the window of a folder workspace
Always hot exit

user closes the window of a non-folder workspace
If app exit: hot exit
Else: prompt

user clicks on File > Close Folder in a folder workspace
Always hot exit

user selects File > Open Folder in a window of a folder workspace
Always hot exit

user selects File > Open Folder in a window of a non-folder workspace
If app exit: hot exit
Else: prompt

@Tyriar
Copy link
Member Author

Tyriar commented Jan 16, 2017

Consider the following as an example user flow with onExitAndWindowClose:

  1. Open folder /foo
  2. Do some work in foo, keep some notes in untitled files, have unsaved text files, etc
  3. Open folder /bar in same window (/foo closes and hot exits)
  4. Some time passes, the main process is shutdown.
  5. VS Code is relaunched but only /bar is restored as it was the last opened folder
  6. When user is ready to resume work on /foo they reopen it and get their backed up untitled and text files restored

Non-folder workspaces act the same as with onExit. We treat them more strictly since it's difficult to identify them and there is no way to restore except for during launch of main process.

@bpasero
Copy link
Member

bpasero commented Jan 16, 2017

@Tyriar got it, I suggest we give it a try. As a mitigation maybe we could eventually add an entry to the File menu "Show previous sessions with backups" (bad name, but you get it) that opens a picker inside the window to select from? If we find out that people are confused by it.

@Tyriar Tyriar modified the milestones: January 2017, Backlog Jan 24, 2017
@Tyriar Tyriar added the verification-needed Verification of issue is requested label Jan 24, 2017
@roblourens roblourens added the verified Verification succeeded label Jan 27, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded workbench-hot-exit Preservation of unsaved changes across restarts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants