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

Ensure that old values aren't cached inappropriately #10965

Conversation

colin-grant-work
Copy link
Contributor

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

What it does

Fixes #10959
Fixes #10865

Previously, the caching mechanism in ValidatedPreferenceProxy could mistakenly retain cached values for language overrides. This PR ensures that the cache is maintained only once the proxy subscribes to preference events and that it is updated for all overrides in case of a preference change.

How to test

  1. Activate and deactivate the editor.minimap.enabled preference. It should immediately take effect in open editors (in all languages) and apply correctly to new editors.
  2. Assert that the new tests pass.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added bug bugs found in the application preferences issues related to preferences labels Mar 30, 2022
@colin-grant-work colin-grant-work force-pushed the bugfix/cached-language-override-values branch from b557657 to ce2e752 Compare March 30, 2022 18:13
@colin-grant-work colin-grant-work marked this pull request as draft March 30, 2022 18:35
@msujew
Copy link
Member

msujew commented Mar 30, 2022

@colin-grant-work I believe this fixes #10959 and #10865?

@colin-grant-work colin-grant-work marked this pull request as ready for review March 30, 2022 18:58
@colin-grant-work
Copy link
Contributor Author

@msujew, thanks for pointing those out, you're quite right.

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.

The changes look good to me 👍

  • the added unit-tests tests pass
  • updating editor preferences now take effect (ex: whitespace, minimap)
  • using language id overrides in preferences also work well (verified with whitespaces)

image

@colin-grant-work colin-grant-work merged commit 14b114c into eclipse-theia:master Mar 31, 2022
@colin-grant-work colin-grant-work deleted the bugfix/cached-language-override-values branch March 31, 2022 14:03
@colin-grant-work colin-grant-work added this to the 1.24.0 milestone Mar 31, 2022
@colin-grant-work colin-grant-work mentioned this pull request Jun 3, 2022
1 task
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 preferences issues related to preferences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Toggle minimap" action does nothing editor: render whitespace preference does not work
3 participants