-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Fix reading VB editor options from ISettingsManager (17.4) #66398
Conversation
We regressed a few VB editor options in 17.4 (PR: dotnet#62759) when we switched reading these options from IVsTextManagerEvents4 to ISettingsManager API. - formatting: "Use Tabs", "Indent Size", "Tab Size". "Smart Indent", - statement completion: "Auto list members", "Hide advanced members", "Parameter information", - settings: "Navigation bar" We don't read their values for VS settings correctly for VB because we use language name "VisualBasic" while the editor uses "Basic" in the option storage name. This does not affect other languages (C#, F#).
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
@tmat in a separate PR, lets consider adding an integration test. With the vscode changes coming as well, this space will see a lot of churn. |
@arkalyanms We do have integration tests. They test setting and reading all options. If, however, the option storage name is wrong they will not fail because the settings manager is just gonna create a new option for that storage name with default value. The test will see what it expects to see. The planned changes shouldn't affect VS. |
This is not implemented in 17.4.5, right? |
@skurth No, it is not in 17.4 |
@skurth Indeed. |
MSRC fix. According to the mail, we should update to 16.5.28. The feature branch is based on dotnet#66423 The last checked in bug fix is dotnet#66359 Since QB doesn't want to approve dotnet#66398 now. I feel we shouldn't wait for that change to merge
We regressed a few VB editor options in 17.4 (PR: #62759) when we switched reading these options from IVsTextManagerEvents4 to ISettingsManager API.
We don't read their values for VS settings correctly for VB because we use language name "VisualBasic" while the editor uses "Basic" in the option storage name.
This does not affect other languages (C#, F#).
Fixes #66325
VS issue: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1717717