Skip to content

Commit

Permalink
Merge pull request #42713 from mavasani/FollowUp
Browse files Browse the repository at this point in the history
Follow-up to #42323
  • Loading branch information
msftbot[bot] authored Mar 24, 2020
2 parents e16d390 + abf3eab commit d2e50fd
Show file tree
Hide file tree
Showing 18 changed files with 45 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public AsyncCompletionData.CompletionStartData InitializeCompletion(
// There could be mixed desired behavior per textView and even per same completion session.
// The right fix would be to send this information as a result of the method.
// Then, the Editor would choose the right behavior for mixed cases.
_textView.Options.GlobalOptions.SetOptionValue(NonBlockingCompletionEditorOption, !document.Project.Solution.Workspace.Options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language));
_textView.Options.GlobalOptions.SetOptionValue(NonBlockingCompletionEditorOption, !document.Project.Solution.Workspace.Options.GetOption(CompletionOptions.BlockForCompletionItems2, service.Language));

// In case of calls with multiple completion services for the same view (e.g. TypeScript and C#), those completion services must not be called simultaneously for the same session.
// Therefore, in each completion session we use a list of commit character for a specific completion service and a specific content type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4125,7 +4125,7 @@ class C

Dim workspace = state.Workspace
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, False)))

state.SendTypeChars("Sys.")
Await state.AssertNoCompletionSession()
Expand All @@ -4147,7 +4147,7 @@ class C

Dim workspace = state.Workspace
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, False)))

state.SendTypeChars("Sys")
Await state.AssertSelectedCompletionItem(displayText:="System")
Expand Down Expand Up @@ -4182,7 +4182,7 @@ class C

Dim workspace = state.Workspace
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, False)))

state.SendTypeChars("Sys")

Expand Down Expand Up @@ -4261,7 +4261,7 @@ class C

Dim workspace = state.Workspace
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, False)))

state.SendTypeChars("Sys")
Dim task1 As Task = Nothing
Expand Down Expand Up @@ -4353,7 +4353,7 @@ class C
' Switch to the non-blocking mode
Dim workspace = state.Workspace
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, False)))

' re-use of TestNoBlockOnCompletionItems1
state.SendTypeChars("Sys.")
Expand All @@ -4373,7 +4373,7 @@ class C

' Switch to the blocking mode
workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
.WithChangedOption(CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, True)))
.WithChangedOption(CompletionOptions.BlockForCompletionItems2, LanguageNames.CSharp, True)))

#Disable Warning BC42358 ' Because this call is not awaited, execution of the current method continues before the call is completed
Task.Run(Function()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ private void VerifyTextualTriggerCharacterWorker(
var position = hostDocument.CursorPosition.Value;
var text = hostDocument.GetTextBuffer().CurrentSnapshot.AsText();
var options = workspace.Options
.WithChangedOption(CompletionOptions.TriggerOnTypingLetters, hostDocument.Project.Language, triggerOnLetter)
.WithChangedOption(CompletionOptions.TriggerOnTypingLetters2, hostDocument.Project.Language, triggerOnLetter)
.WithChangedOption(CompletionOptions.TriggerInArgumentLists, hostDocument.Project.Language, showCompletionInArgumentLists);
var trigger = RoslynCompletion.CompletionTrigger.CreateInsertionTrigger(text[position]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal static bool IsTriggerCharacter(SourceText text, int characterPosition,
return true;
}

if (options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp) && IsStartingNewWord(text, characterPosition))
if (options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp) && IsStartingNewWord(text, characterPosition))
{
return true;
}
Expand Down Expand Up @@ -93,7 +93,7 @@ internal static bool IsTriggerAfterSpaceOrStartOfWordCharacter(SourceText text,
// Bring up on space or at the start of a word.
var ch = text[characterPosition];
return SpaceTypedNotBeforeWord(ch, text, characterPosition) ||
(IsStartingNewWord(text, characterPosition) && options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp));
(IsStartingNewWord(text, characterPosition) && options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp));
}

internal static ImmutableHashSet<char> SpaceTriggerCharacter => ImmutableHashSet.Create(' ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ internal override bool IsInsertionTrigger(SourceText text, int characterPosition
ch == '[' ||
ch == '(' ||
ch == '~' ||
(options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp) && CompletionUtilities.IsStartingNewWord(text, characterPosition));
(options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp) && CompletionUtilities.IsStartingNewWord(text, characterPosition));
}

internal override ImmutableHashSet<char> TriggerCharacters { get; } = ImmutableHashSet.Create(' ', '[', '(', '~');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected override SyntaxToken GetToken(CompletionItem completionItem, SyntaxTre
internal override bool IsInsertionTrigger(SourceText text, int characterPosition, OptionSet options)
{
var ch = text[characterPosition];
return ch == ' ' || (CompletionUtilities.IsStartingNewWord(text, characterPosition) && options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp));
return ch == ' ' || (CompletionUtilities.IsStartingNewWord(text, characterPosition) && options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp));
}

internal override ImmutableHashSet<char> TriggerCharacters { get; } = CompletionUtilities.SpaceTriggerCharacter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal override bool IsInsertionTrigger(SourceText text, int characterPosition
var ch = text[characterPosition];
return ch == ' ' ||
(CompletionUtilities.IsStartingNewWord(text, characterPosition) &&
options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp));
options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp));
}

internal override ImmutableHashSet<char> TriggerCharacters { get; } = CompletionUtilities.SpaceTriggerCharacter;
Expand Down
15 changes: 12 additions & 3 deletions src/Features/Core/Portable/Completion/CompletionOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
storageLocations: new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.TriggerOnTypingLetters"));

#pragma warning disable RS0030 // Do not used banned APIs - Used by TypeScript through IVT, so we cannot change the field type.
public static readonly PerLanguageOption<bool> TriggerOnTypingLetters = (PerLanguageOption<bool>)TriggerOnTypingLetters2!;
#pragma warning restore RS0030 // Do not used banned APIs

public static readonly PerLanguageOption2<bool?> TriggerOnDeletion = new PerLanguageOption2<bool?>(nameof(CompletionOptions), nameof(TriggerOnDeletion), defaultValue: null,
storageLocations: new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.TriggerOnDeletion"));

Expand All @@ -37,9 +42,13 @@ internal static class CompletionOptions
public static readonly PerLanguageOption2<bool> HighlightMatchingPortionsOfCompletionListItems = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(HighlightMatchingPortionsOfCompletionListItems), defaultValue: true,
storageLocations: new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.HighlightMatchingPortionsOfCompletionListItems"));

public static readonly PerLanguageOption2<bool> BlockForCompletionItems = new PerLanguageOption2<bool>(
public static readonly PerLanguageOption2<bool> BlockForCompletionItems2 = new PerLanguageOption2<bool>(
nameof(CompletionOptions), nameof(BlockForCompletionItems), defaultValue: true,
storageLocations: new RoamingProfileStorageLocation($"TextEditor.%LANGUAGE%.Specific.{BlockForCompletionItems}"));
storageLocations: new RoamingProfileStorageLocation($"TextEditor.%LANGUAGE%.Specific.BlockForCompletionItems"));

#pragma warning disable RS0030 // Do not used banned APIs - Used by TypeScript through IVT, so we cannot change the field type.
public static readonly PerLanguageOption<bool> BlockForCompletionItems = (PerLanguageOption<bool>)BlockForCompletionItems2!;
#pragma warning restore RS0030 // Do not used banned APIs

public static readonly PerLanguageOption2<bool> ShowNameSuggestions =
new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(ShowNameSuggestions), defaultValue: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public CompletionOptionsProvider()
public ImmutableArray<IOption> Options { get; } = ImmutableArray.Create<IOption>(
CompletionOptions.HideAdvancedMembers,
CompletionOptions.TriggerOnTyping,
CompletionOptions.TriggerOnTypingLetters,
CompletionOptions.TriggerOnTypingLetters2,
CompletionOptions.ShowCompletionItemFilters,
CompletionOptions.HighlightMatchingPortionsOfCompletionListItems,
CompletionOptions.EnterKeyBehavior,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Completion.Providers
End Function

Private Function IsStartingNewWord(text As SourceText, characterPosition As Integer, options As OptionSet) As Boolean
If Not options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.VisualBasic) Then
If Not options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.VisualBasic) Then
Return False
End If

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Completion.Providers
text(characterPosition) = "("c OrElse
(characterPosition > 1 AndAlso text(characterPosition) = "="c AndAlso text(characterPosition - 1) = ":"c) OrElse
SyntaxFacts.IsIdentifierStartCharacter(text(characterPosition)) AndAlso
options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.VisualBasic)
options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.VisualBasic)
End Function

Friend Overrides ReadOnly Property TriggerCharacters As ImmutableHashSet(Of Char) = ImmutableHashSet.Create(" "c, "("c, "="c)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.FSharp.Completion
internal static class FSharpCompletionOptions
{
// Suppression due to https://github.com/dotnet/roslyn/issues/42614
public static PerLanguageOption<bool> BlockForCompletionItems { get; } = ((PerLanguageOption<bool>)Microsoft.CodeAnalysis.Completion.CompletionOptions.BlockForCompletionItems)!;
public static PerLanguageOption<bool> BlockForCompletionItems { get; } = ((PerLanguageOption<bool>)Microsoft.CodeAnalysis.Completion.CompletionOptions.BlockForCompletionItems2)!;
}
}
4 changes: 2 additions & 2 deletions src/VisualStudio/CSharp/Impl/Options/AutomationObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public int AutoInsertAsteriskForNewLinesOfBlockComments

public int BringUpOnIdentifier
{
get { return GetBooleanOption(CompletionOptions.TriggerOnTypingLetters); }
set { SetBooleanOption(CompletionOptions.TriggerOnTypingLetters, value); }
get { return GetBooleanOption(CompletionOptions.TriggerOnTypingLetters2); }
set { SetBooleanOption(CompletionOptions.TriggerOnTypingLetters2, value); }
}

public int HighlightMatchingPortionsOfCompletionListItems
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public IntelliSenseOptionPageControl(OptionStore optionStore) : base(optionStore
BindToOption(Show_completion_item_filters, CompletionOptions.ShowCompletionItemFilters, LanguageNames.CSharp);
BindToOption(Highlight_matching_portions_of_completion_list_items, CompletionOptions.HighlightMatchingPortionsOfCompletionListItems, LanguageNames.CSharp);

BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptions.TriggerOnTypingLetters, LanguageNames.CSharp);
BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptions.TriggerOnTypingLetters2, LanguageNames.CSharp);
Show_completion_list_after_a_character_is_deleted.IsChecked = this.OptionStore.GetOption(CompletionOptions.TriggerOnDeletion, LanguageNames.CSharp) == true;
Show_completion_list_after_a_character_is_deleted.IsEnabled = Show_completion_list_after_a_character_is_typed.IsChecked == true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Options
MyBase.New(optionStore)
InitializeComponent()

BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptions.TriggerOnTypingLetters, LanguageNames.VisualBasic)
BindToOption(Show_completion_list_after_a_character_is_typed, CompletionOptions.TriggerOnTypingLetters2, LanguageNames.VisualBasic)
Show_completion_list_after_a_character_is_deleted.IsChecked = Me.OptionStore.GetOption(
CompletionOptions.TriggerOnDeletion, LanguageNames.VisualBasic) <> False

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Snippets

Friend Overrides Function IsInsertionTrigger(text As SourceText, characterPosition As Integer, options As OptionSet) As Boolean
Return Char.IsLetterOrDigit(text(characterPosition)) AndAlso
options.GetOption(CompletionOptions.TriggerOnTypingLetters, LanguageNames.VisualBasic)
options.GetOption(CompletionOptions.TriggerOnTypingLetters2, LanguageNames.VisualBasic)
End Function

Friend Overrides ReadOnly Property TriggerCharacters As ImmutableHashSet(Of Char) = ImmutableHashSet(Of Char).Empty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@

namespace Microsoft.CodeAnalysis.Options
{
/// <summary>
/// Internal base option type that is available in both the Workspaces layer and CodeStyle layer.
/// Its definition in Workspaces layer sub-types "IOption" and its definition in CodeStyle layer
/// 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>
internal interface IOption2 : IEquatable<IOption2?>
#if !CODE_STYLE
, IOption
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public bool Equals(OptionDefinition other)
this.Group == other.Group &&
this.IsPerLanguage == other.IsPerLanguage;

// DefaultValue and Type can differ between different but equivalent implementations of "ICodeStyleOption".
// So, we skip these fields for equality checks of code style options.
if (equals && !(this.DefaultValue is ICodeStyleOption))
{
equals = Equals(this.DefaultValue, other.DefaultValue) && this.Type == other.Type;
Expand All @@ -92,6 +94,8 @@ public override int GetHashCode()
hash = unchecked((hash * (int)0xA5555529) + this.Name.GetHashCode());
hash = unchecked((hash * (int)0xA5555529) + this.IsPerLanguage.GetHashCode());

// DefaultValue and Type can differ between different but equivalent implementations of "ICodeStyleOption".
// So, we skip these fields for hash computation of code style options.
if (!(this.DefaultValue is ICodeStyleOption))
{
hash = unchecked((hash * (int)0xA5555529) + this.DefaultValue?.GetHashCode() ?? 0);
Expand Down

0 comments on commit d2e50fd

Please sign in to comment.