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

Notify when deserializing project state for missing directories #15681

Merged
merged 21 commits into from Sep 20, 2017

Conversation

Projects
None yet
2 participants
@smashwilson
Member

smashwilson commented Sep 18, 2017

When deserializing a serialized Project state that references one or more root directories that no longer exist, show an error notification.

When deserializing an unmodified buffer that is no longer accessible for any reason, silently skip that buffer.

When deserializing a modified buffer that is no longer accessible, deserialize the buffer. Eventually we might want to decorate the editor with a banner warning that the underlying file no longer exists.

Fixes #13325.

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Sep 18, 2017

Member

Does this also fix the autosave case? #11797 and duplicate atom/autosave#78

Member

Ben3eeE commented Sep 18, 2017

Does this also fix the autosave case? #11797 and duplicate atom/autosave#78

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Sep 18, 2017

Member

I'll have to check. Thanks for the heads-up 👍

I think at least not opening blank editors when deserializing buffers that are no longer present should help. But, if you modify the blank editor before the reload event is received, I'm guessing that you'd still lose data. 🤔

Member

smashwilson commented Sep 18, 2017

I'll have to check. Thanks for the heads-up 👍

I think at least not opening blank editors when deserializing buffers that are no longer present should help. But, if you modify the blank editor before the reload event is received, I'm guessing that you'd still lose data. 🤔

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Sep 19, 2017

Member

Verification process

Given a project directory tree with a few files.

~/
  fixtures/
    project-1/
      one.txt
      two.txt
    project-2/
      three.txt

Successful serialization and deserialization

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. Open Atom in developer mode.
  • The tree view should show ~/fixtures/project-1 and ~/fixtures/project-2 as root directories.
  • Three editors should be open on one.txt, two.txt, and three.txt.
  • The modifications to one.txt should be preserved.

Partial deserialization failure

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. mv ~/fixtures/project-1 ~/fixtures/project-1-bak
  7. Open Atom in developer mode.
  • The tree view should show ~/fixtures/project-2 as the only open project.
  • A dialog should appear stating that the directory ~/fixtures/project-1 is missing.
  • One editor should be open on three.txt.

Full deserialization failure

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. mv ~/fixtures/project-1 ~/fixtures/project-1-bak
  7. mv ~/fixtures/project-2 ~/fixtures/project-2-bak
  8. Open Atom in developer mode.
  • The tree view should show no open projects.
  • A dialog should appear stating that the directories ~/fixtures/project-1 and ~/fixtures/project-2 are missing.
  • No editors should be opened.
Member

smashwilson commented Sep 19, 2017

Verification process

Given a project directory tree with a few files.

~/
  fixtures/
    project-1/
      one.txt
      two.txt
    project-2/
      three.txt

Successful serialization and deserialization

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. Open Atom in developer mode.
  • The tree view should show ~/fixtures/project-1 and ~/fixtures/project-2 as root directories.
  • Three editors should be open on one.txt, two.txt, and three.txt.
  • The modifications to one.txt should be preserved.

Partial deserialization failure

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. mv ~/fixtures/project-1 ~/fixtures/project-1-bak
  7. Open Atom in developer mode.
  • The tree view should show ~/fixtures/project-2 as the only open project.
  • A dialog should appear stating that the directory ~/fixtures/project-1 is missing.
  • One editor should be open on three.txt.

Full deserialization failure

  1. Open Atom in developer mode with fresh window state.
  2. Add ~/fixtures/project-1 and ~/fixtures/project-2 as project directories.
  3. Open one.txt, two.txt, and three.txt.
  4. Modify one.txt.
  5. Close Atom.
  6. mv ~/fixtures/project-1 ~/fixtures/project-1-bak
  7. mv ~/fixtures/project-2 ~/fixtures/project-2-bak
  8. Open Atom in developer mode.
  • The tree view should show no open projects.
  • A dialog should appear stating that the directories ~/fixtures/project-1 and ~/fixtures/project-2 are missing.
  • No editors should be opened.

smashwilson added some commits Sep 18, 2017

@smashwilson smashwilson merged commit 615b67b into master Sep 20, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
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-unmounted-drive branch Sep 20, 2017

facebook-github-bot added a commit to facebook/nuclide that referenced this pull request Sep 20, 2017

Hack to fix remote buffer serialization on Windows
Summary:
On Windows, it appears that Atom no longer swallows EINVAL errors from attempting to open `nuclide:\a\b\c` on startup (this is an invalid drive identifier). Unfortunately this has the side effect of torpedoing the entire deserialization phase! :(

To work around this for now, we're going to have to monkey-patch the TextBuffer serialization function to sanitize these remote URIs. For now I've decided to replace `nuclide:/` with `nuclide_\` on Windows-only to make sure that the deserialized path is at least valid.

I'm working to try and find the regression within `superstring` that caused this so we can avoid this. Alternatively there's atom/atom#15681 which may make our deserialization experience better as well.

Reviewed By: ebluestein

Differential Revision: D5868633

fbshipit-source-id: 0a09ed468380edb9d17228420fe413dbf200424d

aadisriram added a commit to facebook/nuclide that referenced this pull request Sep 22, 2017

Hack to fix remote buffer serialization on Windows
Summary:
On Windows, it appears that Atom no longer swallows EINVAL errors from attempting to open `nuclide:\a\b\c` on startup (this is an invalid drive identifier). Unfortunately this has the side effect of torpedoing the entire deserialization phase! :(

To work around this for now, we're going to have to monkey-patch the TextBuffer serialization function to sanitize these remote URIs. For now I've decided to replace `nuclide:/` with `nuclide_\` on Windows-only to make sure that the deserialized path is at least valid.

I'm working to try and find the regression within `superstring` that caused this so we can avoid this. Alternatively there's atom/atom#15681 which may make our deserialization experience better as well.

Reviewed By: ebluestein

Differential Revision: D5868633

fbshipit-source-id: 0a09ed468380edb9d17228420fe413dbf200424d

hansonw added a commit to facebook-atom/atom-ide-ui that referenced this pull request Sep 22, 2017

Hack to fix remote buffer serialization on Windows
Summary:
On Windows, it appears that Atom no longer swallows EINVAL errors from attempting to open `nuclide:\a\b\c` on startup (this is an invalid drive identifier). Unfortunately this has the side effect of torpedoing the entire deserialization phase! :(

To work around this for now, we're going to have to monkey-patch the TextBuffer serialization function to sanitize these remote URIs. For now I've decided to replace `nuclide:/` with `nuclide_\` on Windows-only to make sure that the deserialized path is at least valid.

I'm working to try and find the regression within `superstring` that caused this so we can avoid this. Alternatively there's atom/atom#15681 which may make our deserialization experience better as well.

Reviewed By: ebluestein

Differential Revision: D5868633

fbshipit-source-id: 0a09ed468380edb9d17228420fe413dbf200424d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment