Skip to content

Conversation

@tmat
Copy link
Member

@tmat tmat commented Jul 18, 2022

This reverts commit 9c68f9b.

Based on @jasonmalinowski's analysis the RPS regression caused by the original PR is actually an improvement.

@tmat tmat requested a review from a team as a code owner July 18, 2022 22:55
@tmat tmat requested a review from a team July 18, 2022 22:55
@ghost ghost added the Area-IDE label Jul 18, 2022
@tmat tmat enabled auto-merge (squash) July 19, 2022 16:44
Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

Looks like this somehow breaks servicehub.indexingservice, the process no longer launches in RPS tests with this change. See comment in this for more details. Blocking this until we understand what's going on. FYI @davkean

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

The issue with IndexingService is now fixed pending insertion.
https://devdiv.visualstudio.com/DevDiv/_git/VS-Platform/pullrequest/413466

@tmat tmat merged commit efc78a2 into dotnet:main Jul 27, 2022
@ghost ghost added this to the Next milestone Jul 27, 2022
tmat added a commit to tmat/roslyn that referenced this pull request Jul 29, 2022
@tmat tmat deleted the RevRev branch July 29, 2022 19:12
@tmat tmat modified the milestones: Next, 17.4 Aug 29, 2022
tmat added a commit to tmat/roslyn that referenced this pull request Jan 12, 2023
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#).
tmat added a commit to tmat/roslyn that referenced this pull request Jan 12, 2023
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#).
tmat added a commit to tmat/roslyn that referenced this pull request Jan 12, 2023
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#).
tmat added a commit that referenced this pull request Jan 17, 2023
We regressed a few VB editor options in 17.4 (PR: #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#).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants