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

Check application state before showing file resource dialog #10900

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Mar 18, 2022

What it does

Fixes #9843 by checking application state before trying to show a file-resource-related dialog.

I think ideally, it wouldn't be the resource that was responsible for this dialog: different contexts should be allowed to handle it differently. But this fix does address the specific problem described in the bug with a minimum change in the architecture. I'm open to other suggestions (for example, maybe if we're running remotely, we should automatically reject rather than automatically approve the request?).

How to test

  1. Open the application.
  2. Open a binary file or large file of some kind.
  3. Observe that you get a dialog requesting confirmation that you want to open the binary file as text.
  4. Close and reopen the application.
  5. The layout should be restored, and the file should be opened as text without a dialog prompt.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added filesystem issues related to the filesystem editor issues related to the editor shell issues related to the core shell labels Mar 18, 2022
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment: A few of those texts already exist in the nls.metadata.json file.

packages/filesystem/src/browser/file-resource.ts Outdated Show resolved Hide resolved
packages/filesystem/src/browser/file-resource.ts Outdated Show resolved Hide resolved
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that the changes work well for me 👍
I verified by:

  1. starting the application
  2. open a .png with the code editor - prompt is shown
  3. reload the application
  4. the app starts and properly restores the editor - on master the loading animation would go on forever

@colin-grant-work colin-grant-work merged commit 76f8928 into eclipse-theia:master Mar 23, 2022
@colin-grant-work colin-grant-work deleted the bugfix/restore-with-binary branch March 23, 2022 16:09
@colin-grant-work colin-grant-work added this to the 1.24.0 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor issues related to the editor filesystem issues related to the filesystem shell issues related to the core shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout cannot restored when a large binary file is opened.
3 participants