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

[theming] changing themes quickly results in a dialog #6786

Closed
vince-fugnitto opened this issue Dec 23, 2019 · 6 comments · Fixed by #7110
Closed

[theming] changing themes quickly results in a dialog #6786

vince-fugnitto opened this issue Dec 23, 2019 · 6 comments · Fixed by #7110
Assignees
Labels
bug bugs found in the application theming issues related to theming

Comments

@vince-fugnitto
Copy link
Member

Description

Using #6475, if a user changes themes quickly, a dialog is displayed that there is inconsistency in the global settings.json in the editor and on the filesystem (auto-save is off)

image

Steps to Reproduce

  1. using VS Code like theming (colors and icons) #6475, attempt to change the color theme quickly.
  2. a dialog appears that there are unsaved changes in the settings.json.
@vince-fugnitto vince-fugnitto added the theming issues related to theming label Dec 23, 2019
@akosyakov akosyakov added the bug bugs found in the application label Dec 26, 2019
@akosyakov
Copy link
Member

I don't think it can happen often, does it? I had hard time to reproduce it.

@akosyakov
Copy link
Member

but it breaks tests from time to time, i.e. #5774 failing because of it

@akosyakov akosyakov self-assigned this Dec 30, 2019
@akosyakov
Copy link
Member

Oh, basically we need to reimplement logic from the monaco editor model to handle concurrent updates for resource. I think we are better to rewrite AbstractResourcePreferenceProvider with the monaco editor model instead of using the resource directly as @vince-fugnitto were suggesting in another PR. In such implementation if there is no opened editors for such resource we will trigger auto save, otherwise rely on a user to save the dirty preference editor.

@akosyakov
Copy link
Member

That's tricky:

  • the editor model cannot be acquired if editor preferences are not ready, so there is a cycle
  • it breaks preference tests, since it requires Monaco

I think we can do it after #6409 that we can port tests to run within the app.

@akosyakov
Copy link
Member

KeymapsService suffers from the same problems

@akosyakov
Copy link
Member

It should be fixed with #7110. @vince-fugnitto could you try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application theming issues related to theming
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants