Skip to content

Fix VisualStudioSettingsOptionPersister issues and increase test coverage#67064

Merged
tmat merged 9 commits into
dotnet:mainfrom
tmat:VSOptionStorage
Mar 1, 2023
Merged

Fix VisualStudioSettingsOptionPersister issues and increase test coverage#67064
tmat merged 9 commits into
dotnet:mainfrom
tmat:VSOptionStorage

Conversation

@tmat

@tmat tmat commented Feb 26, 2023

Copy link
Copy Markdown
Member

Fixes a couple of issues, hardens the implementation and adds more test coverage.

The first issue is that when reading fallback option flags we have not used the default values for flags that were not stored in the VS settings storage if there was at least one flag that was stored. As a result these flags defaulted to 0, which was incorrect for new lines after braces options.

The second issue occurred when VS settings manager sync'd options from online storage and triggered option change event. We read incorrect value of the option (individual bool flag, instead of combined flag enum) and stored it in the global options, which later resulted in InvalidCastException.

Fixes AB#1751694.
Fixes AB#1743163.
Fixes AB#1733803

@tmat tmat force-pushed the VSOptionStorage branch 4 times, most recently from d3ffef1 to c1560df Compare February 27, 2023 23:32
@tmat tmat marked this pull request as ready for review February 28, 2023 23:58
@tmat tmat requested review from a team as code owners February 28, 2023 23:58
@tmat tmat force-pushed the VSOptionStorage branch from 567e73e to 3b1abe9 Compare March 1, 2023 00:02
Comment thread src/Workspaces/CoreTestUtilities/Options/OptionsTestHelpers.cs Outdated
Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
Comment thread src/VisualStudio/Core/Def/Options/VisualStudioSettingsOptionPersister.cs Outdated
@tmat tmat enabled auto-merge (squash) March 1, 2023 18:07
@tmat tmat merged commit 7f065ab into dotnet:main Mar 1, 2023
@ghost ghost added this to the Next milestone Mar 1, 2023
@RikkiGibson RikkiGibson modified the milestones: Next, 17.6 P2 Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants