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

Re-add ServiceFeatureOnOffOptions.ClosedFileDiagnostic to fix the bre… #39955

Merged
merged 2 commits into from
Nov 22, 2019

Conversation

mavasani
Copy link
Contributor

…ak for TS and F#

Recent PR #39699 introduced a breaking API and functionality change for TypeScript and F# as they are using our internal option ClosedFileDiagnostic for controlling their full solution analysis experience. This PR restores this option and functionality for non-C#/VB languages. Additionally, the newly added option SolutionCrawlerOptions.BackgroundAnalysisScope has been made a per-language option so that all languages can have different UI/setting for this option.

…ak for TS and F#

Recent PR dotnet#39699 introduced a breaking API and functionality change for TypeScript and F# as they are using our internal option ClosedFileDiagnostics for controlling their full solution analysis experience. This PR restores the ClosedFileDiagnostics option and functionality for non C#/VB languages. Additionally, the newly added option SolutionCrawlerOptions.BackgroundAnalysisScope has been made a per-language option so that all languages can have different UI/setting for this option.
@mavasani mavasani added this to the 16.5 milestone Nov 22, 2019
@mavasani mavasani requested review from jasonmalinowski, JoeRobich, genlu and a team November 22, 2019 00:11
@mavasani mavasani requested a review from a team as a code owner November 22, 2019 00:11
{
case LanguageNames.CSharp:
case LanguageNames.VisualBasic:
return options.GetOption(BackgroundAnalysisScopeOption, language);
Copy link
Member

Choose a reason for hiding this comment

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

So are we ignoring previous settings of the open/closed diagnostics setting for C# and VB? We don't want to read those if the new one hasn't been set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no UI to set the older option for C# and VB though. Also, I am not sure how would we hook up option page setting based on 2 options. Current BindToOption approach simply connects the UI radio button to the new option. Having said that, we can potentially set this up somehow, but I am not sure if the effort is worth it.

@mavasani
Copy link
Contributor Author

Merging this to unblock insertion. I will address any additional feedback with a follow-up PR.

@mavasani mavasani merged commit 6f0b3d6 into dotnet:master Nov 22, 2019
@mavasani mavasani deleted the FixIVTBreak branch November 22, 2019 03:16
mavasani added a commit to mavasani/roslyn that referenced this pull request Nov 22, 2019
dotnet#39955 re-added the option back to Workspaces instead of Features, its original location
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