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

Improve behavior when restoring session that references a missing project folder #18742

Merged
merged 6 commits into from Jan 24, 2019

Conversation

Projects
None yet
2 participants
@smashwilson
Copy link
Member

smashwilson commented Jan 24, 2019

Requirements for Contributing a Bug Fix

Identify the Bug

Fixes #16645.

Description of the Change

When Atom is launched from the dock or start menu, or from the command-line with no path arguments, it (by default) attempts to restore the previous open windows and session state by reading a file at ~/.atom/storage/application.json. I've modified the logic to handle root directories listed there that are no longer present by:

  • Omitting them from the deserialized window state;
  • Displaying a one-time warning listing the directories that were unable to load; and
  • Continuing to use now-missing root directories to derive the state key used to identify persisted project state that should be loaded.

I've done this by:

  1. Introducing a foldersToOpen parameter to AtomApplication::openLocations(). These are paths that must be existing directories, in contrast to pathsToOpen, which may be files or directories that exist or not.
  2. Translating this to locationsToOpen with a new key, mustBeDirectory: true.
  3. Understanding mustBeDirectory in AtomEnvironment::openLocations() where we actually do the stat() calls. Locations with mustBeDirectory: true that are either missing or now files are added to an Array.
  4. Concatenating foldersToOpen and missingFolders to derive the initial state key.
  5. Display a (grammatically correct ) warning about the folders that are now gone.

Alternate Designs

I thought about introducing some new syntax for pathsToOpen to indicate that a specific path is required to be a directory, like +some/path/. It would have saved me adding another argument to our already-busy open methods, but I didn't want to have more magic in our command-line story.

Possible Drawbacks

If a user has different stored session state for the set of project folders that are still present, it'll be clobbered by this session.

Because of the way session state is tracked, we can only save one in this case. I chose to go with the session state that the user explicitly requested (assuming they understand that project folder set = session state key, of course).

Verification Process

  1. Open Atom with two project folders, folder-0 and folder-1.
  2. Open a buffer within folder-0 and make unsaved changes.
  3. Completely close Atom.
  4. Delete folder-1.
  5. Open Atom with no arguments (atom).

Atom should open with folder-0 as an open project folder, an open buffer with the unsaved changes, and a warning notification saying that folder-1 is no longer present.

Release Notes

  • When restoring state, project folders that no longer exist will cause a one-time notification and be removed.

smashwilson added some commits Jan 24, 2019

@smashwilson smashwilson requested a review from daviwil Jan 24, 2019

@daviwil
Copy link
Member

daviwil left a comment

Looks great man! :shipit:

description += 'directory `'
description += missingFolders[0].pathToOpen
description += '` does not exist.'
} else if (missingFolders.length === 2) {

This comment has been minimized.

@daviwil

daviwil Jan 24, 2019

Member

Not a big deal, but you could probably just include this case into the else by trimming the , off of the end of the description string when length === 2 before adding the "and" part, but tomato tomahto :)

@smashwilson smashwilson merged commit 6055f28 into master Jan 24, 2019

3 checks passed

Atom Pull Requests #20190124.7 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@smashwilson smashwilson deleted the aw/deleted-project-folder branch Jan 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.