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

Follow-up to #42323 #42713

Merged
3 commits merged into from
Mar 24, 2020
Merged

Follow-up to #42323 #42713

3 commits merged into from
Mar 24, 2020

Conversation

mavasani
Copy link
Contributor

Address pending PR feedback from #42323

Verified that the first commit resolves the binary breaking change for TS. This commit renames the completion options used by TS to have the 2 suffix for internal use in Roslyn and restores the original option with public type for consumption by TS. Once TS moves to external access model, we can move the field with public type to TS layer, similar to the approach taken for all F# options.

@mavasani mavasani added this to the 16.6 milestone Mar 24, 2020
@mavasani mavasani requested review from jasonmalinowski, sharwell and a team March 24, 2020 04:43
@mavasani mavasani requested a review from a team as a code owner March 24, 2020 04:43
@@ -17,8 +17,13 @@ internal static class CompletionOptions
// This is serialized by the Visual Studio-specific LanguageSettingsPersister
public static readonly PerLanguageOption2<bool> TriggerOnTyping = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTyping), defaultValue: true);

public static readonly PerLanguageOption2<bool> TriggerOnTypingLetters = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTypingLetters), defaultValue: true,
public static readonly PerLanguageOption2<bool> TriggerOnTypingLetters2 = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTypingLetters), defaultValue: true,
Copy link
Contributor Author

@mavasani mavasani Mar 24, 2020

Choose a reason for hiding this comment

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

Ensured that the name argument passed to the option constructor was not changed.

/// explicitly defines all the members from "IOption" type as "IOption" is not available in CodeStyle layer.
/// This ensures that all the sub-types of <see cref="IOption2"/> in either layer see an identical
/// set of interface members.
/// </summary>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this is really helpful.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit d2e50fd into dotnet:master Mar 24, 2020
@mavasani mavasani deleted the FollowUp branch March 24, 2020 11:57
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants