From 8a0e438a23a3e47bbea6597ef4b16351a599abda Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 28 Nov 2016 23:45:25 -0800 Subject: [PATCH 01/19] Refactor completion code for handling typed characters. --- .../Completion/Controller_TypeChar.cs | 191 ++++++++++-------- 1 file changed, 104 insertions(+), 87 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 9b25358a4d8a9..8903d48a6ede2 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -128,108 +128,125 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg Contract.ThrowIfNull(options); var isTextuallyTriggered = IsTextualTriggerCharacter(completionService, args.TypedChar, options); - var isPotentialFilterCharacter = IsPotentialFilterCharacter(args); var trigger = CompletionTrigger.CreateInsertionTrigger(args.TypedChar); if (sessionOpt == null) { - // No computation at all. If this is not a trigger character, we just ignore it and - // stay in this state. Otherwise, if it's a trigger character, start up a new - // computation and start computing the model in the background. - if (isTextuallyTriggered) - { - // First create the session that represents that we now have a potential - // completion list. Then tell it to start computing. - StartNewModelComputation(completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); - return; - } - else - { - // No need to do anything. Just stay in the state where we have no session. - return; - } + // No current session. start one if necessary. + ExecuteTYpeCharWithNoSession(completionService, isTextuallyTriggered, trigger); } else { - sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); + ExecuteTypeCharWithSession(args, nextHandler, initialTextSnapshot, initialCaretPosition, completionService, options, isTextuallyTriggered, trigger); + } + } - // If the session is up, it may be in one of many states. It may know nothing - // (because it is currently computing the list of completions). Or it may have a - // list of completions that it has filtered. + private void ExecuteTypeCharWithSession( + TypeCharCommandArgs args, + Action nextHandler, + ITextSnapshot initialTextSnapshot, + SnapshotPoint initialCaretPosition, + CompletionService completionService, + OptionSet options, + bool isTextuallyTriggered, + CompletionTrigger trigger) + { - // If the user types something which is absolutely known to be a filter character - // then we can just proceed without blocking. - if (isPotentialFilterCharacter) - { - if (isTextuallyTriggered) - { - // The character typed was something like "a". It can both filter a list if - // we have computed one, or it can trigger a new list. Ask the computation - // to compute again. If nothing has been computed, then it will try to - // compute again, otherwise it will just ignore this request. - sessionOpt.ComputeModel(completionService, trigger, _roles, options); - } + sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); - // Now filter whatever result we have. - sessionOpt.FilterModel( - CompletionFilterReason.TypeChar, - recheckCaretPosition: false, - dismissIfEmptyAllowed: true, - filterState: null); - } - else + // If the session is up, it may be in one of many states. It may know nothing + // (because it is currently computing the list of completions). Or it may have a + // list of completions that it has filtered. + + // If the user types something which is absolutely known to be a filter character + // then we can just proceed without blocking. + if (IsPotentialFilterCharacter(args)) + { + if (isTextuallyTriggered) { - // It wasn't a trigger or filter character. At this point, we make our - // determination on what to do based on what has actually been computed and - // what's being typed. This means waiting on the session and will effectively - // block the user. - - // Again, from this point on we must block on the computation to decide what to - // do. - - // What they type may end up filtering, committing, or else will dismiss. - // - // For example, we may filter in cases like this: "Color." - // - // "Color" will have already filtered the list down to some things like - // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we - // actually want to filter some more. But we can't know that ahead of time until - // we have computed the list of completions. - if (this.IsFilterCharacter(args.TypedChar)) - { - // Known to be a filter character for the currently selected item. So just - // filter the session. - sessionOpt.FilterModel(CompletionFilterReason.TypeChar, - recheckCaretPosition: false, - dismissIfEmptyAllowed: true, - filterState: null); - return; - } + // The character typed was something like "a". It can both filter a list if + // we have computed one, or it can trigger a new list. Ask the computation + // to compute again. If nothing has been computed, then it will try to + // compute again, otherwise it will just ignore this request. + sessionOpt.ComputeModel(completionService, trigger, _roles, options); + } - // It wasn't a filter character. We'll either commit what's selected, or we'll - // dismiss the completion list. First, ensure that what was typed is in the - // buffer. + // Now filter whatever result we have. + sessionOpt.FilterModel( + CompletionFilterReason.TypeChar, + recheckCaretPosition: false, + dismissIfEmptyAllowed: true, + filterState: null); + return; + } - // Now, commit if it was a commit character. - if (!this.CommitIfCommitCharacter(args.TypedChar, initialTextSnapshot, nextHandler)) - { - // Wasn't a filter or commit character. Stop what we're doing as we have - // no idea what this is. - this.StopModelComputation(); - } + // It wasn't a trigger or filter character. At this point, we make our + // determination on what to do based on what has actually been computed and + // what's being typed. This means waiting on the session and will effectively + // block the user. - // The character may commit/dismiss and then trigger completion again. So check - // for that here. + // Again, from this point on we must block on the computation to decide what to + // do. - if (isTextuallyTriggered) - { - // First create the session that represents that we now have a potential - // completion list. - StartNewModelComputation( - completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); - return; - } - } + // What they type may end up filtering, committing, or else will dismiss. + // + // For example, we may filter in cases like this: "Color." + // + // "Color" will have already filtered the list down to some things like + // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we + // actually want to filter some more. But we can't know that ahead of time until + // we have computed the list of completions. + if (this.IsFilterCharacter(args.TypedChar)) + { + // Known to be a filter character for the currently selected item. So just + // filter the session. + sessionOpt.FilterModel(CompletionFilterReason.TypeChar, + recheckCaretPosition: false, + dismissIfEmptyAllowed: true, + filterState: null); + return; + } + + // It wasn't a filter character. We'll either commit what's selected, or we'll + // dismiss the completion list. First, ensure that what was typed is in the + // buffer. + + // Now, commit if it was a commit character. + if (!this.CommitIfCommitCharacter(args.TypedChar, initialTextSnapshot, nextHandler)) + { + // Wasn't a filter or commit character. Stop what we're doing as we have + // no idea what this is. + this.StopModelComputation(); + } + + // The character may commit/dismiss and then trigger completion again. So check + // for that here. + + if (isTextuallyTriggered) + { + // First create the session that represents that we now have a potential + // completion list. + StartNewModelComputation( + completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); + } + } + + private void ExecuteTYpeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) + { + // No computation at all. If this is not a trigger character, we just ignore it and + // stay in this state. Otherwise, if it's a trigger character, start up a new + // computation and start computing the model in the background. + if (isTextuallyTriggered) + { + // First create the session that represents that we now have a potential + // completion list. Then tell it to start computing. + StartNewModelComputation(completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); + return; + } + else + { + // No need to do anything. Just stay in the state where we have no session. + return; } } From 8fa8bee37ee17af8bf9fbbecb95bbbcc3a5b2a16 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 00:32:48 -0800 Subject: [PATCH 02/19] Make blocking behavior configurable in completion. --- .../Completion/Controller_TypeChar.cs | 83 ++++++++++++------- .../CSharpCompletionCommandHandlerTests.vb | 52 ++++++++++++ .../Portable/Completion/CompletionOptions.cs | 4 + .../Portable/Completion/CompletionService.cs | 11 +-- 4 files changed, 114 insertions(+), 36 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 8903d48a6ede2..083241c62893b 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using Microsoft.CodeAnalysis.Completion; using Microsoft.CodeAnalysis.Editor.Commands; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; @@ -133,7 +134,7 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg if (sessionOpt == null) { // No current session. start one if necessary. - ExecuteTYpeCharWithNoSession(completionService, isTextuallyTriggered, trigger); + ExecuteTypeCharWithNoSession(completionService, isTextuallyTriggered, trigger); } else { @@ -180,13 +181,32 @@ private void ExecuteTypeCharWithSession( return; } - // It wasn't a trigger or filter character. At this point, we make our - // determination on what to do based on what has actually been computed and - // what's being typed. This means waiting on the session and will effectively - // block the user. + // It wasn't a trigger or filter character. At this point, we make our determination + // on what to do based on what has actually been computed and what's being typed. + // In order to do this, we need to know what the actual completion items are. If we + // want these, we have to block. We will do this if the language allows this. + // Generally, the language will allow this if completions can be computed quickly. + // If they can't, then we will not block and we will stop computed immediately. - // Again, from this point on we must block on the computation to decide what to - // do. + if (sessionOpt.InitialUnfilteredModel == null) + { + // We haven't finished computing completion items. Block if the language wants + // us to. Otherwise, dismiss the session + var result = Workspace.TryGetWorkspace(this.SubjectBuffer.AsTextContainer(), out var workspace); + Debug.Assert(result, "We successfully got the completion service. We must be able to get to the workspace."); + + if (!options.GetOption(CompletionOptions.BlockForCompletionItems, completionService.Language)) + { + // Language doesn't want us to block. Just stop our existing session as we + // don't have enough data on what to do. Restart the session if appropriate + // for the character typed. + DismissSessionAndStartAnotherIfTriggered(completionService, isTextuallyTriggered, trigger); + return; + } + } + + // We've either computed the items, or the language is fine with blocking. + // Block to get the model and determine what we should so. // What they type may end up filtering, committing, or else will dismiss. // @@ -196,7 +216,8 @@ private void ExecuteTypeCharWithSession( // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we // actually want to filter some more. But we can't know that ahead of time until // we have computed the list of completions. - if (this.IsFilterCharacter(args.TypedChar)) + var model = sessionOpt.WaitForModel(); + if (this.IsFilterCharacter(args.TypedChar, model)) { // Known to be a filter character for the currently selected item. So just // filter the session. @@ -208,19 +229,23 @@ private void ExecuteTypeCharWithSession( } // It wasn't a filter character. We'll either commit what's selected, or we'll - // dismiss the completion list. First, ensure that what was typed is in the - // buffer. - - // Now, commit if it was a commit character. - if (!this.CommitIfCommitCharacter(args.TypedChar, initialTextSnapshot, nextHandler)) - { - // Wasn't a filter or commit character. Stop what we're doing as we have - // no idea what this is. - this.StopModelComputation(); - } + // dismiss the completion list. + this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler); + + // If the character was a commit character, then we will have committed the list + // and dismissed the session. If it wasn't a commit character, then we don't know + // what to do with this character, and we want to dismiss the session. So, at this + // point we always want hte session dismissed. 'DismissSessionAndStartAnotherIfTriggered' + // will ensure that happens. + + // The character may have committed *and* should start a new session. i.e. + // hittins "Sys". should commit "System" and start a new session. + DismissSessionAndStartAnotherIfTriggered(completionService, isTextuallyTriggered, trigger); + } - // The character may commit/dismiss and then trigger completion again. So check - // for that here. + private void DismissSessionAndStartAnotherIfTriggered(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) + { + DismissSessionIfActive(); if (isTextuallyTriggered) { @@ -231,7 +256,7 @@ private void ExecuteTypeCharWithSession( } } - private void ExecuteTYpeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) + private void ExecuteTypeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) { // No computation at all. If this is not a trigger character, we just ignore it and // stay in this state. Otherwise, if it's a trigger character, start up a new @@ -307,12 +332,10 @@ private bool IsTextualTriggerCharacter(CompletionService completionService, char return completionService.ShouldTriggerCompletion(previousPosition.Snapshot.AsText(), caretPosition, trigger, _roles, options); } - private bool IsCommitCharacter(char ch, out Model model) + private bool IsCommitCharacter(char ch, Model model) { AssertIsForeground(); - // TODO(cyrusn): Find a way to allow the user to cancel out of this. - model = sessionOpt.WaitForModel(); if (model == null || model.IsSoftSelection) { return false; @@ -374,12 +397,10 @@ internal static bool IsCommitCharacter( return completionRules.DefaultCommitCharacters.IndexOf(ch) >= 0; } - private bool IsFilterCharacter(char ch) + private bool IsFilterCharacter(char ch, Model model) { AssertIsForeground(); - // TODO(cyrusn): Find a way to allow the user to cancel out of this. - var model = sessionOpt.WaitForModel(); if (model == null) { return false; @@ -451,13 +472,19 @@ private string GetTextTypedSoFar(Model model, CompletionItem selectedItem) private bool CommitIfCommitCharacter( char ch, ITextSnapshot initialTextSnapshot, Action nextHandler) + { + return CommitIfCommitCharacter(ch, sessionOpt.WaitForModel(), initialTextSnapshot, nextHandler); + } + + private bool CommitIfCommitCharacter( + char ch, Model model, ITextSnapshot initialTextSnapshot, Action nextHandler) { AssertIsForeground(); // Note: this function is called after the character has already been inserted into the // buffer. - if (!IsCommitCharacter(ch, out var model)) + if (!IsCommitCharacter(ch, model)) { return false; } diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb index 94ade0e871520..ec8826d225f14 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb @@ -2599,5 +2599,57 @@ class C Await state.AssertSelectedCompletionItem(displayText:="Console", isHardSelected:=True) End Using End Function + + + Public Async Function TestNoBlockOnCompletionItems1() As Task + Dim tcs = New TaskCompletionSource(Of Boolean) + Using state = TestState.CreateCSharpTestState( + + using $$ + , {New TaskControlledCompletionProvider(tcs.Task)}) + + state.Workspace.Options = state.Workspace.Options.WithChangedOption( + CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False) + + state.SendTypeChars("Sys.") + Await state.WaitForAsynchronousOperationsAsync() + Await state.AssertNoCompletionSession() + Assert.Contains("Sys.", state.GetLineTextFromCaretPosition()) + + tcs.SetResult(True) + End Using + End Function + + + Public Async Function TestNoBlockOnCompletionItems2() As Task + Using state = TestState.CreateCSharpTestState( + + using $$ + , {New TaskControlledCompletionProvider(Task.FromResult(True))}) + + state.Workspace.Options = state.Workspace.Options.WithChangedOption( + CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False) + + state.SendTypeChars("Sys") + Await state.WaitForAsynchronousOperationsAsync() + Await state.AssertCompletionSession() + state.SendTypeChars(".") + Assert.Contains("System.", state.GetLineTextFromCaretPosition()) + End Using + End Function + + Private Class TaskControlledCompletionProvider + Inherits CompletionProvider + + Private ReadOnly _task As Task + + Public Sub New(task As Task) + _task = task + End Sub + + Public Overrides Function ProvideCompletionsAsync(context As CompletionContext) As Task + Return _task + End Function + End Class End Class End Namespace \ No newline at end of file diff --git a/src/Features/Core/Portable/Completion/CompletionOptions.cs b/src/Features/Core/Portable/Completion/CompletionOptions.cs index f79bd9d111dc9..31c7153b92b0f 100644 --- a/src/Features/Core/Portable/Completion/CompletionOptions.cs +++ b/src/Features/Core/Portable/Completion/CompletionOptions.cs @@ -33,6 +33,10 @@ internal static class CompletionOptions public static readonly PerLanguageOption HighlightMatchingPortionsOfCompletionListItems = new PerLanguageOption(nameof(CompletionOptions), nameof(HighlightMatchingPortionsOfCompletionListItems), defaultValue: true, storageLocations: new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.HighlightMatchingPortionsOfCompletionListItems")); + public static readonly PerLanguageOption BlockForCompletionItems = new PerLanguageOption( + nameof(CompletionOptions), nameof(BlockForCompletionItems), defaultValue: true, + storageLocations: new RoamingProfileStorageLocation($"TextEditor.%LANGUAGE%.Specific.{BlockForCompletionItems}")); + public static IEnumerable> GetDev15CompletionOptions() { yield return ShowCompletionItemFilters; diff --git a/src/Features/Core/Portable/Completion/CompletionService.cs b/src/Features/Core/Portable/Completion/CompletionService.cs index e5cb5d09c6452..4b150f2e3800d 100644 --- a/src/Features/Core/Portable/Completion/CompletionService.cs +++ b/src/Features/Core/Portable/Completion/CompletionService.cs @@ -3,11 +3,11 @@ using System; using System.Collections.Immutable; using System.Globalization; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Completion @@ -22,9 +22,7 @@ public abstract class CompletionService : ILanguageService /// Gets the service corresponding to the specified document. /// public static CompletionService GetService(Document document) - { - return document.Project.LanguageServices.GetService(); - } + => document.GetLanguageService(); /// /// The language from this service corresponds to. @@ -34,10 +32,7 @@ public static CompletionService GetService(Document document) /// /// Gets the current presentation and behavior rules. /// - public virtual CompletionRules GetRules() - { - return CompletionRules.Default; - } + public virtual CompletionRules GetRules() => CompletionRules.Default; /// /// Returns true if the character recently inserted or deleted in the text should trigger completion. From bf947c4607f6ed2383ad8979091c9a31fa35b85f Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 00:54:14 -0800 Subject: [PATCH 03/19] Provide a common entrypoint with which to get the model. --- .../IntelliSense/AbstractController.cs | 2 +- .../Completion/Controller.Session_Wait.cs | 12 +++- .../IntelliSense/Completion/Controller.cs | 11 ++++ ...ntroller_CommitUniqueCompletionListItem.cs | 2 +- .../Completion/Controller_ReturnKey.cs | 4 +- .../Completion/Controller_TabKey.cs | 2 +- .../Completion/Controller_TypeChar.cs | 57 +++++++------------ 7 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/AbstractController.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/AbstractController.cs index 908b4e7ca14b0..01b9d07b0a3af 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/AbstractController.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/AbstractController.cs @@ -26,7 +26,7 @@ internal abstract class AbstractController sessionOpt != null; public AbstractController(ITextView textView, ITextBuffer subjectBuffer, IIntelliSensePresenter presenter, IAsynchronousOperationListener asyncListener, IDocumentProvider documentProvider, string asyncOperationId) { diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs index 6f6812b0dc6a9..34807f9f1702e 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs @@ -10,10 +10,20 @@ internal partial class Controller { internal partial class Session { - internal Model WaitForModel() + internal Model WaitForModel(bool blockForInitialItems) { AssertIsForeground(); + if (!blockForInitialItems && this.InitialUnfilteredModel == null) + { + // If we don't have our initial completion items, and the caller doesn't want + // us to block, then just return nothing here. + return null; + } + + // Otherwise, if we have at least computed items, or our all caller is ok with + // blocking, then wait until all work is done. + using (Logger.LogBlock(FunctionId.Completion_ModelComputation_WaitForModel, CancellationToken.None)) { return Computation.ModelTask.WaitAndGetResult(CancellationToken.None); diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index 2014521f8faa7..2acad4a4b9c9a 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -97,6 +97,17 @@ private SnapshotPoint GetCaretPointInSubjectBuffer() return this.TextView.BufferGraph.MapUpOrDownToBuffer(this.TextView.Caret.Position.BufferPosition, this.SubjectBuffer).GetValueOrDefault(); } + private Model WaitForModel() + { + var model = sessionOpt.WaitForModel(BlockForCompletionItems()); + if (model == null) + { + DismissSessionIfActive(); + } + + return model; + } + internal override void OnModelUpdated(Model modelOpt) { AssertIsForeground(); diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs index a65796fc1b0f2..66a904a581c35 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs @@ -35,7 +35,7 @@ void ICommandHandler.ExecuteCommand( } // Get the selected item. If it's unique, then we want to commit it. - var model = this.sessionOpt.WaitForModel(); + var model = WaitForModel(); if (model == null) { // Computation failed. Just pass this command on. diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs index 22a9285aff900..209fe2c9939aa 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs @@ -48,7 +48,8 @@ private void CommitOnEnter(out bool sendThrough, out bool committed) { AssertIsForeground(); - var model = sessionOpt.WaitForModel(); + var service = GetCompletionService(); + var model = WaitForModel(); // If there's no model, then there's nothing to commit. if (model == null) @@ -88,7 +89,6 @@ private void CommitOnEnter(out bool sendThrough, out bool committed) var textTypedSoFar = model.GetCurrentTextInSnapshot( viewSpan, this.TextView.TextSnapshot, this.GetCaretPointInViewBuffer()); - var service = GetCompletionService(); sendThrough = SendEnterThroughToEditor( service.GetRules(), model.SelectedItem, textTypedSoFar); } diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs index 95efb9b712bda..f64ba406f0402 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs @@ -155,7 +155,7 @@ private void CommitOnTab(out bool committed) { AssertIsForeground(); - var model = sessionOpt.WaitForModel(); + var model = WaitForModel(); // If there's no model, then there's nothing to commit. if (model == null) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 083241c62893b..49ab41ee40f91 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -188,26 +188,6 @@ private void ExecuteTypeCharWithSession( // Generally, the language will allow this if completions can be computed quickly. // If they can't, then we will not block and we will stop computed immediately. - if (sessionOpt.InitialUnfilteredModel == null) - { - // We haven't finished computing completion items. Block if the language wants - // us to. Otherwise, dismiss the session - var result = Workspace.TryGetWorkspace(this.SubjectBuffer.AsTextContainer(), out var workspace); - Debug.Assert(result, "We successfully got the completion service. We must be able to get to the workspace."); - - if (!options.GetOption(CompletionOptions.BlockForCompletionItems, completionService.Language)) - { - // Language doesn't want us to block. Just stop our existing session as we - // don't have enough data on what to do. Restart the session if appropriate - // for the character typed. - DismissSessionAndStartAnotherIfTriggered(completionService, isTextuallyTriggered, trigger); - return; - } - } - - // We've either computed the items, or the language is fine with blocking. - // Block to get the model and determine what we should so. - // What they type may end up filtering, committing, or else will dismiss. // // For example, we may filter in cases like this: "Color." @@ -216,7 +196,7 @@ private void ExecuteTypeCharWithSession( // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we // actually want to filter some more. But we can't know that ahead of time until // we have computed the list of completions. - var model = sessionOpt.WaitForModel(); + var model = WaitForModel(); if (this.IsFilterCharacter(args.TypedChar, model)) { // Known to be a filter character for the currently selected item. So just @@ -230,22 +210,10 @@ private void ExecuteTypeCharWithSession( // It wasn't a filter character. We'll either commit what's selected, or we'll // dismiss the completion list. - this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler); - - // If the character was a commit character, then we will have committed the list - // and dismissed the session. If it wasn't a commit character, then we don't know - // what to do with this character, and we want to dismiss the session. So, at this - // point we always want hte session dismissed. 'DismissSessionAndStartAnotherIfTriggered' - // will ensure that happens. - - // The character may have committed *and* should start a new session. i.e. - // hittins "Sys". should commit "System" and start a new session. - DismissSessionAndStartAnotherIfTriggered(completionService, isTextuallyTriggered, trigger); - } - - private void DismissSessionAndStartAnotherIfTriggered(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) - { - DismissSessionIfActive(); + if (!this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler)) + { + this.StopModelComputation(); + } if (isTextuallyTriggered) { @@ -256,6 +224,19 @@ private void DismissSessionAndStartAnotherIfTriggered(CompletionService completi } } + private bool BlockForCompletionItems() + { + var service = GetCompletionService(); + var options = GetOptions(); + return service != null && options != null && + options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); + } + + private void DismissSessionAndStartAnotherIfTriggered(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) + { + + } + private void ExecuteTypeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) { // No computation at all. If this is not a trigger character, we just ignore it and @@ -473,7 +454,7 @@ private string GetTextTypedSoFar(Model model, CompletionItem selectedItem) private bool CommitIfCommitCharacter( char ch, ITextSnapshot initialTextSnapshot, Action nextHandler) { - return CommitIfCommitCharacter(ch, sessionOpt.WaitForModel(), initialTextSnapshot, nextHandler); + return this.CommitIfCommitCharacter(ch, this.WaitForModel(), initialTextSnapshot, nextHandler); } private bool CommitIfCommitCharacter( From 6e03d6cb36dc0cdef8af3df4570f214bf22f8d3b Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 00:56:09 -0800 Subject: [PATCH 04/19] Add comment --- .../Core/Implementation/IntelliSense/Completion/Controller.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index 2acad4a4b9c9a..b82d1ee30117a 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -102,6 +102,9 @@ private Model WaitForModel() var model = sessionOpt.WaitForModel(BlockForCompletionItems()); if (model == null) { + // We either didn't get a model because we blocked, and no model was computed, + // or because we didn't block, and no initial model was computed yet. In either + // event, make sure we have no more active session. DismissSessionIfActive(); } From 0e4044ca8ade5405b3d219731c127d277938c8ad Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 01:30:17 -0800 Subject: [PATCH 05/19] Commit-Unique-Items asynchronously if the language does not want to block. --- .../IntelliSense/Completion/Controller.cs | 2 - ...ntroller_CommitUniqueCompletionListItem.cs | 41 ++++++++++++++ .../Completion/Controller_TypeChar.cs | 14 ++--- .../CSharpCompletionCommandHandlerTests.vb | 54 +++++++++++++++++++ 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index b82d1ee30117a..ede3f0f4e2650 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -3,8 +3,6 @@ using System.Collections.Immutable; using Microsoft.CodeAnalysis.Completion; using Microsoft.CodeAnalysis.Editor.Commands; -using Microsoft.CodeAnalysis.Editor.Host; -using Microsoft.CodeAnalysis.Editor.Options; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs index 66a904a581c35..e24b1ddf73e2b 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.Editor.Commands; +using Microsoft.CodeAnalysis.Shared.TestHooks; namespace Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.Completion { @@ -34,6 +37,12 @@ void ICommandHandler.ExecuteCommand( } } + if (sessionOpt.InitialUnfilteredModel == null && !BlockForCompletionItems()) + { + CommitUniqueCompletionListItemAsynchronously(); + return; + } + // Get the selected item. If it's unique, then we want to commit it. var model = WaitForModel(); if (model == null) @@ -43,6 +52,38 @@ void ICommandHandler.ExecuteCommand( return; } + CommitIfUnique(model); + } + + private void CommitUniqueCompletionListItemAsynchronously() + { + // We're in a language that doesn't want to block, but hasn't computed the initial + // set of completion items. In this case, we asynchronously wait for the items to + // be computed. And if nothing has happened between now and that point, we proceed + // with committing the items. + var currentSession = sessionOpt; + var currentTask = currentSession.Computation.ModelTask; + + // We're kicking off async work. Track this with an async token for test purposes. + var token = ((IController)this).BeginAsyncOperation(nameof(CommitUniqueCompletionListItemAsynchronously)); + + var task = currentTask.ContinueWith(t => + { + this.AssertIsForeground(); + + if (this.sessionOpt == currentSession && + this.sessionOpt.Computation.ModelTask == currentTask) + { + // Nothing happened between when we were invoked and now. + CommitIfUnique(t.Result); + } + }, CancellationToken.None, TaskContinuationOptions.OnlyOnRanToCompletion, this.ForegroundTaskScheduler); + + task.CompletesAsyncOperation(token); + } + + private void CommitIfUnique(Model model) + { // Note: Dev10 behavior seems to be that if there is no unique item that filtering is // turned off. However, i do not know if this is desirable behavior, or merely a bug // with how that convoluted code worked. So I'm not maintaining that behavior here. If diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 49ab41ee40f91..25dddcf5e474b 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -208,12 +208,14 @@ private void ExecuteTypeCharWithSession( return; } - // It wasn't a filter character. We'll either commit what's selected, or we'll - // dismiss the completion list. - if (!this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler)) - { - this.StopModelComputation(); - } + // It wasn't a filter character. It's either a commit character, or something we + // don't know how to handle. First try to commit. + this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler); + + // At this point we don't want a session anymore (either because we committed, or + // because we got a character we don't know how to handle). Unilaterally dismiss + // the session. + DismissSessionIfActive(); if (isTextuallyTriggered) { diff --git a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb index ec8826d225f14..0b71df0c96915 100644 --- a/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb +++ b/src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb @@ -2638,6 +2638,60 @@ class C End Using End Function + + Public Async Function TestNoBlockOnCompletionItems4() As Task + Dim tcs = New TaskCompletionSource(Of Boolean) + Using state = TestState.CreateCSharpTestState( + + using $$ + , {New TaskControlledCompletionProvider(tcs.Task)}) + + state.Workspace.Options = state.Workspace.Options.WithChangedOption( + CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False) + + state.SendTypeChars("Sys") + state.SendCommitUniqueCompletionListItem() + Await Task.Delay(250) + Await state.AssertNoCompletionSession(block:=False) + Assert.Contains("Sys", state.GetLineTextFromCaretPosition()) + Assert.DoesNotContain("System", state.GetLineTextFromCaretPosition()) + + tcs.SetResult(True) + + Await state.WaitForAsynchronousOperationsAsync() + Await state.AssertNoCompletionSession() + Assert.Contains("System", state.GetLineTextFromCaretPosition()) + End Using + End Function + + + Public Async Function TestNoBlockOnCompletionItems3() As Task + Dim tcs = New TaskCompletionSource(Of Boolean) + Using state = TestState.CreateCSharpTestState( + + using $$ + , {New TaskControlledCompletionProvider(tcs.Task)}) + + state.Workspace.Options = state.Workspace.Options.WithChangedOption( + CompletionOptions.BlockForCompletionItems, LanguageNames.CSharp, False) + + state.SendTypeChars("Sys") + state.SendCommitUniqueCompletionListItem() + Await Task.Delay(250) + Await state.AssertNoCompletionSession(block:=False) + Assert.Contains("Sys", state.GetLineTextFromCaretPosition()) + Assert.DoesNotContain("System", state.GetLineTextFromCaretPosition()) + + state.SendTypeChars("a") + + tcs.SetResult(True) + + Await state.WaitForAsynchronousOperationsAsync() + Await state.AssertCompletionSession() + Assert.Contains("Sysa", state.GetLineTextFromCaretPosition()) + End Using + End Function + Private Class TaskControlledCompletionProvider Inherits CompletionProvider From 808e8ad93220535ba28bc6e37dc4cad352ca1fb3 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 01:39:59 -0800 Subject: [PATCH 06/19] Move local. --- .../IntelliSense/Completion/Controller_ReturnKey.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs index 209fe2c9939aa..06cd2a69e4374 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs @@ -48,7 +48,6 @@ private void CommitOnEnter(out bool sendThrough, out bool committed) { AssertIsForeground(); - var service = GetCompletionService(); var model = WaitForModel(); // If there's no model, then there's nothing to commit. @@ -89,6 +88,7 @@ private void CommitOnEnter(out bool sendThrough, out bool committed) var textTypedSoFar = model.GetCurrentTextInSnapshot( viewSpan, this.TextView.TextSnapshot, this.GetCaretPointInViewBuffer()); + var service = GetCompletionService(); sendThrough = SendEnterThroughToEditor( service.GetRules(), model.SelectedItem, textTypedSoFar); } From 53c573d901992bcb20ce627fe7f59e69c53c7187 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 01:43:27 -0800 Subject: [PATCH 07/19] Wrapping. --- .../IntelliSense/Completion/Controller_TypeChar.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 25dddcf5e474b..d76decb97c50d 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -143,14 +143,9 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg } private void ExecuteTypeCharWithSession( - TypeCharCommandArgs args, - Action nextHandler, - ITextSnapshot initialTextSnapshot, - SnapshotPoint initialCaretPosition, - CompletionService completionService, - OptionSet options, - bool isTextuallyTriggered, - CompletionTrigger trigger) + TypeCharCommandArgs args, Action nextHandler, ITextSnapshot initialTextSnapshot, + SnapshotPoint initialCaretPosition, CompletionService completionService, + OptionSet options, bool isTextuallyTriggered, CompletionTrigger trigger) { sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); From 6235c1944428441f62ac682e355122954a3e3eb9 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 01:44:26 -0800 Subject: [PATCH 08/19] remove method. --- .../IntelliSense/Completion/Controller_TypeChar.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index d76decb97c50d..9c00e30066daf 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -147,7 +147,6 @@ private void ExecuteTypeCharWithSession( SnapshotPoint initialCaretPosition, CompletionService completionService, OptionSet options, bool isTextuallyTriggered, CompletionTrigger trigger) { - sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); // If the session is up, it may be in one of many states. It may know nothing @@ -229,11 +228,6 @@ private bool BlockForCompletionItems() options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); } - private void DismissSessionAndStartAnotherIfTriggered(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) - { - - } - private void ExecuteTypeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) { // No computation at all. If this is not a trigger character, we just ignore it and From 9503ad88af13217a66551a9396db16158d6c7db0 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 11:42:23 -0800 Subject: [PATCH 09/19] Restore code to original form. --- .../IntelliSense/Completion/Controller.cs | 2 +- ...ntroller_CommitUniqueCompletionListItem.cs | 2 +- .../Completion/Controller_TypeChar.cs | 204 +++++++++--------- 3 files changed, 100 insertions(+), 108 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index ede3f0f4e2650..7a435d8dbd617 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -97,7 +97,7 @@ private SnapshotPoint GetCaretPointInSubjectBuffer() private Model WaitForModel() { - var model = sessionOpt.WaitForModel(BlockForCompletionItems()); + var model = sessionOpt.WaitForModel(ShouldBlockForCompletionItems()); if (model == null) { // We either didn't get a model because we blocked, and no model was computed, diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs index e24b1ddf73e2b..01c691c0f7dcf 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs @@ -37,7 +37,7 @@ void ICommandHandler.ExecuteCommand( } } - if (sessionOpt.InitialUnfilteredModel == null && !BlockForCompletionItems()) + if (sessionOpt.InitialUnfilteredModel == null && !ShouldBlockForCompletionItems()) { CommitUniqueCompletionListItemAsynchronously(); return; diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 9c00e30066daf..d3736a6b6a2f8 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -97,18 +97,25 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg // that goes into the other side of the seam, the character may be a commit character. // If it's a commit character, just commit without trying to check caret position, // since the caret is no longer in our buffer. - if (isOnSeam && this.CommitIfCommitCharacter(args.TypedChar, initialTextSnapshot, nextHandler)) + if (isOnSeam) { - return; + var model = this.WaitForModel(); + if (this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler)) + { + return; + } } if (_autoBraceCompletionChars.Contains(args.TypedChar) && - this.SubjectBuffer.GetFeatureOnOffOption(InternalFeatureOnOffOptions.AutomaticPairCompletion) && - this.CommitIfCommitCharacter(args.TypedChar, initialTextSnapshot, nextHandler)) + this.SubjectBuffer.GetFeatureOnOffOption(InternalFeatureOnOffOptions.AutomaticPairCompletion)) { - // I don't think there is any better way than this. if typed char is one of auto brace completion char, - // we don't do multiple buffer change check - return; + var model = this.WaitForModel(); + if (this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler)) + { + // I don't think there is any better way than this. if typed char is one of auto brace completion char, + // we don't do multiple buffer change check + return; + } } // If we were computing anything, we stop. We only want to process a typechar @@ -129,98 +136,108 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg Contract.ThrowIfNull(options); var isTextuallyTriggered = IsTextualTriggerCharacter(completionService, args.TypedChar, options); + var isPotentialFilterCharacter = IsPotentialFilterCharacter(args); var trigger = CompletionTrigger.CreateInsertionTrigger(args.TypedChar); if (sessionOpt == null) { - // No current session. start one if necessary. - ExecuteTypeCharWithNoSession(completionService, isTextuallyTriggered, trigger); + // No computation at all. If this is not a trigger character, we just ignore it and + // stay in this state. Otherwise, if it's a trigger character, start up a new + // computation and start computing the model in the background. + if (isTextuallyTriggered) + { + // First create the session that represents that we now have a potential + // completion list. Then tell it to start computing. + StartNewModelComputation(completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); + return; + } + else + { + // No need to do anything. Just stay in the state where we have no session. + return; + } } else { - ExecuteTypeCharWithSession(args, nextHandler, initialTextSnapshot, initialCaretPosition, completionService, options, isTextuallyTriggered, trigger); - } - } + sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); - private void ExecuteTypeCharWithSession( - TypeCharCommandArgs args, Action nextHandler, ITextSnapshot initialTextSnapshot, - SnapshotPoint initialCaretPosition, CompletionService completionService, - OptionSet options, bool isTextuallyTriggered, CompletionTrigger trigger) - { - sessionOpt.UpdateModelTrackingSpan(initialCaretPosition); - - // If the session is up, it may be in one of many states. It may know nothing - // (because it is currently computing the list of completions). Or it may have a - // list of completions that it has filtered. + // If the session is up, it may be in one of many states. It may know nothing + // (because it is currently computing the list of completions). Or it may have a + // list of completions that it has filtered. - // If the user types something which is absolutely known to be a filter character - // then we can just proceed without blocking. - if (IsPotentialFilterCharacter(args)) - { - if (isTextuallyTriggered) + // If the user types something which is absolutely known to be a filter character + // then we can just proceed without blocking. + if (isPotentialFilterCharacter) { - // The character typed was something like "a". It can both filter a list if - // we have computed one, or it can trigger a new list. Ask the computation - // to compute again. If nothing has been computed, then it will try to - // compute again, otherwise it will just ignore this request. - sessionOpt.ComputeModel(completionService, trigger, _roles, options); - } - - // Now filter whatever result we have. - sessionOpt.FilterModel( - CompletionFilterReason.TypeChar, - recheckCaretPosition: false, - dismissIfEmptyAllowed: true, - filterState: null); - return; - } + if (isTextuallyTriggered) + { + // The character typed was something like "a". It can both filter a list if + // we have computed one, or it can trigger a new list. Ask the computation + // to compute again. If nothing has been computed, then it will try to + // compute again, otherwise it will just ignore this request. + sessionOpt.ComputeModel(completionService, trigger, _roles, options); + } - // It wasn't a trigger or filter character. At this point, we make our determination - // on what to do based on what has actually been computed and what's being typed. - // In order to do this, we need to know what the actual completion items are. If we - // want these, we have to block. We will do this if the language allows this. - // Generally, the language will allow this if completions can be computed quickly. - // If they can't, then we will not block and we will stop computed immediately. + // Now filter whatever result we have. + sessionOpt.FilterModel( + CompletionFilterReason.TypeChar, + recheckCaretPosition: false, + dismissIfEmptyAllowed: true, + filterState: null); + } + else + { + // It wasn't a trigger or filter character. At this point, we make our + // determination on what to do based on what has actually been computed and + // what's being typed. This means waiting on the session and will effectively + // block the user. + + var model = WaitForModel(); + + // What they type may end up filtering, committing, or else will dismiss. + // + // For example, we may filter in cases like this: "Color." + // + // "Color" will have already filtered the list down to some things like + // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we + // actually want to filter some more. But we can't know that ahead of time until + // we have computed the list of completions. + if (this.IsFilterCharacter(args.TypedChar, model)) + { + // Known to be a filter character for the currently selected item. So just + // filter the session. + sessionOpt.FilterModel(CompletionFilterReason.TypeChar, + recheckCaretPosition: false, + dismissIfEmptyAllowed: true, + filterState: null); + return; + } - // What they type may end up filtering, committing, or else will dismiss. - // - // For example, we may filter in cases like this: "Color." - // - // "Color" will have already filtered the list down to some things like - // "Color", "Color.Red", "Color.Blue", etc. When we process the 'dot', we - // actually want to filter some more. But we can't know that ahead of time until - // we have computed the list of completions. - var model = WaitForModel(); - if (this.IsFilterCharacter(args.TypedChar, model)) - { - // Known to be a filter character for the currently selected item. So just - // filter the session. - sessionOpt.FilterModel(CompletionFilterReason.TypeChar, - recheckCaretPosition: false, - dismissIfEmptyAllowed: true, - filterState: null); - return; - } + // It wasn't a filter character. We'll either commit what's selected, or we'll + // dismiss the completion list. First, ensure that what was typed is in the + // buffer. - // It wasn't a filter character. It's either a commit character, or something we - // don't know how to handle. First try to commit. - this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler); + // Now, commit if it was a commit character. + this.CommitIfCommitCharacter(args.TypedChar, model, initialTextSnapshot, nextHandler); - // At this point we don't want a session anymore (either because we committed, or - // because we got a character we don't know how to handle). Unilaterally dismiss - // the session. - DismissSessionIfActive(); + // At this point we don't want a session anymore (either because we committed, or + // because we got a character we don't know how to handle). Unilaterally dismiss + // the session. + DismissSessionIfActive(); - if (isTextuallyTriggered) - { - // First create the session that represents that we now have a potential - // completion list. - StartNewModelComputation( - completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); + // The character may commit/dismiss and then trigger completion again. So check + // for that here. + if (isTextuallyTriggered) + { + StartNewModelComputation( + completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); + return; + } + } } } - private bool BlockForCompletionItems() + private bool ShouldBlockForCompletionItems() { var service = GetCompletionService(); var options = GetOptions(); @@ -228,25 +245,6 @@ private bool BlockForCompletionItems() options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); } - private void ExecuteTypeCharWithNoSession(CompletionService completionService, bool isTextuallyTriggered, CompletionTrigger trigger) - { - // No computation at all. If this is not a trigger character, we just ignore it and - // stay in this state. Otherwise, if it's a trigger character, start up a new - // computation and start computing the model in the background. - if (isTextuallyTriggered) - { - // First create the session that represents that we now have a potential - // completion list. Then tell it to start computing. - StartNewModelComputation(completionService, trigger, filterItems: true, dismissIfEmptyAllowed: true); - return; - } - else - { - // No need to do anything. Just stay in the state where we have no session. - return; - } - } - private bool IsOnSeam() { var caretPoint = TextView.Caret.Position.Point; @@ -442,12 +440,6 @@ private string GetTextTypedSoFar(Model model, CompletionItem selectedItem) return filterText; } - private bool CommitIfCommitCharacter( - char ch, ITextSnapshot initialTextSnapshot, Action nextHandler) - { - return this.CommitIfCommitCharacter(ch, this.WaitForModel(), initialTextSnapshot, nextHandler); - } - private bool CommitIfCommitCharacter( char ch, Model model, ITextSnapshot initialTextSnapshot, Action nextHandler) { From d8e62a86b6b7748c9642a36544d8cd2a77195167 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 11:44:18 -0800 Subject: [PATCH 10/19] Name methods to prevent accidental misusage. --- .../IntelliSense/Completion/Controller.Session_Wait.cs | 5 ++++- .../Implementation/IntelliSense/Completion/Controller.cs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs index 34807f9f1702e..5ddbf227c0cfa 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs @@ -10,7 +10,10 @@ internal partial class Controller { internal partial class Session { - internal Model WaitForModel(bool blockForInitialItems) + /// + /// Should only be called from . + /// + internal Model WaitForModel_DoNotCallDirectly(bool blockForInitialItems) { AssertIsForeground(); diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index 7a435d8dbd617..a7587814e4f13 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -97,7 +97,7 @@ private SnapshotPoint GetCaretPointInSubjectBuffer() private Model WaitForModel() { - var model = sessionOpt.WaitForModel(ShouldBlockForCompletionItems()); + var model = sessionOpt.WaitForModel_DoNotCallDirectly(ShouldBlockForCompletionItems()); if (model == null) { // We either didn't get a model because we blocked, and no model was computed, From f7a8827a58ba5081169d0ddaf3926111e095a89f Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 11:45:09 -0800 Subject: [PATCH 11/19] Move comment. --- .../Controller_CommitUniqueCompletionListItem.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs index 01c691c0f7dcf..13706783b1ca2 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs @@ -39,6 +39,10 @@ void ICommandHandler.ExecuteCommand( if (sessionOpt.InitialUnfilteredModel == null && !ShouldBlockForCompletionItems()) { + // We're in a language that doesn't want to block, but hasn't computed the initial + // set of completion items. In this case, we asynchronously wait for the items to + // be computed. And if nothing has happened between now and that point, we proceed + // with committing the items. CommitUniqueCompletionListItemAsynchronously(); return; } @@ -57,10 +61,6 @@ void ICommandHandler.ExecuteCommand( private void CommitUniqueCompletionListItemAsynchronously() { - // We're in a language that doesn't want to block, but hasn't computed the initial - // set of completion items. In this case, we asynchronously wait for the items to - // be computed. And if nothing has happened between now and that point, we proceed - // with committing the items. var currentSession = sessionOpt; var currentTask = currentSession.Computation.ModelTask; From 97d450e6e692bba15f33105c6d09569813564e42 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 11:46:53 -0800 Subject: [PATCH 12/19] Add paranoia check. --- .../Controller_CommitUniqueCompletionListItem.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs index 13706783b1ca2..9eaab8acfc21e 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CommitUniqueCompletionListItem.cs @@ -47,7 +47,9 @@ void ICommandHandler.ExecuteCommand( return; } - // Get the selected item. If it's unique, then we want to commit it. + // We're either in a language that is ok with blocking, or we have the initial set + // of items. Wait until we're done filtering them, then get the selected item. If + // it's unique, then we want to commit it. var model = WaitForModel(); if (model == null) { @@ -84,6 +86,13 @@ private void CommitUniqueCompletionListItemAsynchronously() private void CommitIfUnique(Model model) { + this.AssertIsForeground(); + + if (model == null) + { + return; + } + // Note: Dev10 behavior seems to be that if there is no unique item that filtering is // turned off. However, i do not know if this is desirable behavior, or merely a bug // with how that convoluted code worked. So I'm not maintaining that behavior here. If From d4951b9fed9c43826ff09e1b5f22de74ef4a2ea0 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 11:48:15 -0800 Subject: [PATCH 13/19] Move method. --- .../Implementation/IntelliSense/Completion/Controller.cs | 8 ++++++++ .../IntelliSense/Completion/Controller_TypeChar.cs | 8 -------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index a7587814e4f13..4287d40bdf266 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -95,6 +95,14 @@ private SnapshotPoint GetCaretPointInSubjectBuffer() return this.TextView.BufferGraph.MapUpOrDownToBuffer(this.TextView.Caret.Position.BufferPosition, this.SubjectBuffer).GetValueOrDefault(); } + private bool ShouldBlockForCompletionItems() + { + var service = GetCompletionService(); + var options = GetOptions(); + return service != null && options != null && + options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); + } + private Model WaitForModel() { var model = sessionOpt.WaitForModel_DoNotCallDirectly(ShouldBlockForCompletionItems()); diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index d3736a6b6a2f8..8460872b72b6f 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -237,14 +237,6 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg } } - private bool ShouldBlockForCompletionItems() - { - var service = GetCompletionService(); - var options = GetOptions(); - return service != null && options != null && - options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); - } - private bool IsOnSeam() { var caretPoint = TextView.Caret.Position.Point; From 189c84241c6e12bdd4256d713c8ac6e0a52a49f2 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 23:02:01 -0800 Subject: [PATCH 14/19] Default to blocking if we can't get options. --- .../Implementation/IntelliSense/Completion/Controller.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index 4287d40bdf266..8cf6c84bb279d 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -99,8 +99,12 @@ private bool ShouldBlockForCompletionItems() { var service = GetCompletionService(); var options = GetOptions(); - return service != null && options != null && - options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); + if (service == null || options == null) + { + return true; + } + + return options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language); } private Model WaitForModel() From abe4917679a02980e035be76ad5528db00163b2f Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 23:51:00 -0800 Subject: [PATCH 15/19] Simplify code handling completion dismissal. --- .../IntelliSense/Completion/Controller.cs | 10 ++++---- .../Controller_AutomaticLineEnder.cs | 2 +- .../Completion/Controller_Backspace.cs | 2 +- .../Controller_CaretPositionChanged.cs | 2 +- .../Completion/Controller_Commit.cs | 2 +- .../Completion/Controller_CutPaste.cs | 13 ++--------- .../Controller_InvokeCompletionList.cs | 5 +--- .../Controller_OnTextViewBufferPostChanged.cs | 2 +- .../Completion/Controller_ReturnKey.cs | 7 ++---- .../Completion/Controller_SnippetCommands.cs | 6 +---- .../Completion/Controller_TabKey.cs | 23 ++++++++++--------- .../Completion/Controller_TypeChar.cs | 2 +- 12 files changed, 29 insertions(+), 47 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index 8cf6c84bb279d..da08e33e188fd 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -126,7 +126,7 @@ internal override void OnModelUpdated(Model modelOpt) AssertIsForeground(); if (modelOpt == null) { - this.StopModelComputation(); + this.DismissSessionIfActive(); } else { @@ -254,13 +254,13 @@ private void CommitItem(CompletionItem item) var model = sessionOpt.InitialUnfilteredModel; // If the selected item is the builder, there's not actually any work to do to commit - if (item == model.SuggestionModeItem) + if (item != model.SuggestionModeItem) { - this.StopModelComputation(); - return; + this.CommitOnNonTypeChar(item, this.sessionOpt.Computation.InitialUnfilteredModel); } - this.CommitOnNonTypeChar(item, this.sessionOpt.Computation.InitialUnfilteredModel); + // Make sure we're always dismissed after any commit request. + this.DismissSessionIfActive(); } private const int MaxMRUSize = 10; diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_AutomaticLineEnder.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_AutomaticLineEnder.cs index 6d12edd9a101b..4a2fc45f058b9 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_AutomaticLineEnder.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_AutomaticLineEnder.cs @@ -29,7 +29,7 @@ void ICommandHandler.ExecuteCommand(AutomaticLine // We did not commit based on enter. So our computation will still be running. Stop it now. if (!committed) { - this.StopModelComputation(); + this.DismissSessionIfActive(); nextHandler(); } } diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Backspace.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Backspace.cs index 69556705648a4..119fd01f9d3ea 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Backspace.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Backspace.cs @@ -98,7 +98,7 @@ private void ExecuteBackspaceOrDelete(ITextView textView, Action nextHandler, bo (model != null && model.OriginalList.Rules.DismissIfLastCharacterDeleted && AllFilterTextsEmpty(model, GetCaretPointInViewBuffer()))) { // If the caret moved out of bounds of our items, then we want to dismiss the list. - this.StopModelComputation(); + this.DismissSessionIfActive(); return; } else if (model != null) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CaretPositionChanged.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CaretPositionChanged.cs index 8b732be2a92fc..b7dcda6b08aaa 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CaretPositionChanged.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CaretPositionChanged.cs @@ -29,7 +29,7 @@ internal override void OnCaretPositionChanged(object sender, EventArgs args) { // Completions hadn't even been computed yet. Just cancel everything we're doing // and move to the Inactive state. - this.StopModelComputation(); + this.DismissSessionIfActive(); return; } diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Commit.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Commit.cs index 8cda5e3958d3c..f79d35272320b 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Commit.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_Commit.cs @@ -49,7 +49,7 @@ private void Commit( // TODO(cyrusn): We still have a general reentrancy problem where calling into a custom // commit provider (or just calling into the editor) may cause something to call back // into us. However, for now, we just hope that no such craziness will occur. - this.StopModelComputation(); + this.DismissSessionIfActive(); CompletionChange completionChange; using (var transaction = CaretPreservingEditTransaction.TryCreate( diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CutPaste.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CutPaste.cs index 1c3f07753ea5c..06794cbad3d9e 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CutPaste.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CutPaste.cs @@ -18,7 +18,7 @@ CommandState ICommandHandler.GetCommandState(CutCommandArgs args void ICommandHandler.ExecuteCommand(CutCommandArgs args, Action nextHandler) { AssertIsForeground(); - EnsureCompletionSessionStopped(); + DismissSessionIfActive(); nextHandler(); } @@ -31,17 +31,8 @@ CommandState ICommandHandler.GetCommandState(PasteCommandArgs void ICommandHandler.ExecuteCommand(PasteCommandArgs args, Action nextHandler) { AssertIsForeground(); - EnsureCompletionSessionStopped(); + DismissSessionIfActive(); nextHandler(); } - - private void EnsureCompletionSessionStopped() - { - AssertIsForeground(); - if (this.sessionOpt != null) - { - StopModelComputation(); - } - } } } diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_InvokeCompletionList.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_InvokeCompletionList.cs index 0662322449374..b249545c53358 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_InvokeCompletionList.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_InvokeCompletionList.cs @@ -21,10 +21,7 @@ void ICommandHandler.ExecuteCommand(InvokeCompl // First, always dismiss whatever session we might have already had. We no longer need // it. - if (sessionOpt != null) - { - this.StopModelComputation(); - } + DismissSessionIfActive(); // Next create the session that represents that we now have a potential completion list. // Then tell it to start computing. diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_OnTextViewBufferPostChanged.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_OnTextViewBufferPostChanged.cs index da43b92abc58a..e095a8b3ff052 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_OnTextViewBufferPostChanged.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_OnTextViewBufferPostChanged.cs @@ -25,7 +25,7 @@ internal override void OnTextViewBufferPostChanged(object sender, EventArgs e) if (model != null && this.IsCaretOutsideAllItemBounds(model, this.GetCaretPointInViewBuffer())) { // If the caret moved out of bounds of our items, then we want to dismiss the list. - this.StopModelComputation(); + this.DismissSessionIfActive(); } else { diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs index 06cd2a69e4374..3db3e877f084e 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs @@ -27,11 +27,8 @@ void ICommandHandler.ExecuteCommand(ReturnKeyCommandArgs a CommitOnEnter(out var sendThrough, out var committed); - // We did not commit based on enter. So our computation will still be running. Stop it now. - if (!committed) - { - this.StopModelComputation(); - } + // Always stop completion after enter has been typed. + DismissSessionIfActive(); // Enter has different behavior amongst languages, so we need to actually defer to // the individual language item to determine what to do. For example, in VB, enter diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_SnippetCommands.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_SnippetCommands.cs index 35c25a441aebf..d054c971a2b21 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_SnippetCommands.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_SnippetCommands.cs @@ -41,11 +41,7 @@ void ICommandHandler.ExecuteCommand(SurroundWithCommand private void DismissCompletionForSnippetPicker(Action nextHandler) { - if (sessionOpt != null) - { - StopModelComputation(); - } - + DismissSessionIfActive(); nextHandler(); } } diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs index f64ba406f0402..68e5c1527705f 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs @@ -37,17 +37,19 @@ void ICommandHandler.ExecuteCommand(TabKeyCommandArgs args, A nextHandler(); return; } + // We are computing a model. Try to commit the selected item if there was one. Note: If // it was able to commit, then we never send the tab to the buffer. That way, if the // user does an undo they'll get to the code they had *before* they hit tab. If the // session wasn't able to commit, then we do send the tab through to the buffer. - CommitOnTab(out var committed); + var committed = CommitOnTab(); + + this.DismissSessionIfActive(); // We did not commit based on tab. So our computation will still be running. Stop it now. // Also, send the tab through to the editor. if (!committed) { - this.StopModelComputation(); nextHandler(); } } @@ -151,7 +153,10 @@ private bool QuestionMarkIsPrecededByIdentifierAndWhitespace( return current == questionPosition; } - private void CommitOnTab(out bool committed) + /// + /// Returns whether or not the item was actually committed. + /// + private bool CommitOnTab() { AssertIsForeground(); @@ -160,20 +165,16 @@ private void CommitOnTab(out bool committed) // If there's no model, then there's nothing to commit. if (model == null) { - committed = false; - return; + return false; } // If the selected item is the builder, there's not actually any work to do to commit - if (model.SelectedItem == model.SuggestionModeItem) + if (model.SelectedItem != model.SuggestionModeItem) { - committed = true; - this.StopModelComputation(); - return; + CommitOnNonTypeChar(model.SelectedItem, model); } - CommitOnNonTypeChar(model.SelectedItem, model); - committed = true; + return true; } } } \ No newline at end of file diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs index 8460872b72b6f..23079fa3deaad 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TypeChar.cs @@ -120,7 +120,7 @@ void ICommandHandler.ExecuteCommand(TypeCharCommandArgs arg // If we were computing anything, we stop. We only want to process a typechar // if it was a normal character. - this.StopModelComputation(); + this.DismissSessionIfActive(); } return; From 0e0730889238365797d630e0f5be03f58de66f90 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 23:55:00 -0800 Subject: [PATCH 16/19] Dismiss when we don't get completion items and we're in a language that doesn't want to block. --- .../IntelliSense/Completion/Controller.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs index da08e33e188fd..b427c3bde2ec9 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.cs @@ -109,12 +109,17 @@ private bool ShouldBlockForCompletionItems() private Model WaitForModel() { - var model = sessionOpt.WaitForModel_DoNotCallDirectly(ShouldBlockForCompletionItems()); - if (model == null) + this.AssertIsForeground(); + + var shouldBlock = ShouldBlockForCompletionItems(); + var model = sessionOpt.WaitForModel_DoNotCallDirectly(shouldBlock); + if (model == null && !shouldBlock) { - // We either didn't get a model because we blocked, and no model was computed, - // or because we didn't block, and no initial model was computed yet. In either - // event, make sure we have no more active session. + // We didn't get a model back, and we're a language that doesn't want to block + // when this happens. Essentially, the user typed something like a commit + // character before we got any results back. In this case, because we're not + // willing to block, we just stop everything that we're doing and return to + // the non-active state. DismissSessionIfActive(); } From d8c7bf4b6837078c5628ec0df08bcba3a2acd4a9 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Tue, 29 Nov 2016 23:57:41 -0800 Subject: [PATCH 17/19] Add comments. --- .../IntelliSense/Completion/Controller_TabKey.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs index 68e5c1527705f..a29332512a92a 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs @@ -44,10 +44,10 @@ void ICommandHandler.ExecuteCommand(TabKeyCommandArgs args, A // session wasn't able to commit, then we do send the tab through to the buffer. var committed = CommitOnTab(); + // After tab, we always want to be in an inactive state. this.DismissSessionIfActive(); - // We did not commit based on tab. So our computation will still be running. Stop it now. - // Also, send the tab through to the editor. + // We did not commit based on tab, so send the tab through to the editor. if (!committed) { nextHandler(); From f86b4cfcaa3281260eb4e1ef792a1132b3d6810c Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 30 Nov 2016 00:00:07 -0800 Subject: [PATCH 18/19] Simplify code. --- .../Completion/Controller_TabKey.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs index a29332512a92a..7c21bad2d2e1a 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs @@ -42,16 +42,10 @@ void ICommandHandler.ExecuteCommand(TabKeyCommandArgs args, A // it was able to commit, then we never send the tab to the buffer. That way, if the // user does an undo they'll get to the code they had *before* they hit tab. If the // session wasn't able to commit, then we do send the tab through to the buffer. - var committed = CommitOnTab(); + CommitOnTab(nextHandler); // After tab, we always want to be in an inactive state. this.DismissSessionIfActive(); - - // We did not commit based on tab, so send the tab through to the editor. - if (!committed) - { - nextHandler(); - } } private bool TryInvokeSnippetCompletion(TabKeyCommandArgs args) @@ -156,16 +150,18 @@ private bool QuestionMarkIsPrecededByIdentifierAndWhitespace( /// /// Returns whether or not the item was actually committed. /// - private bool CommitOnTab() + private void CommitOnTab(Action nextHandler) { AssertIsForeground(); var model = WaitForModel(); - // If there's no model, then there's nothing to commit. + // If there's no model, then there's nothing to commit. So send the tab + // through to the editor. if (model == null) { - return false; + nextHandler(); + return; } // If the selected item is the builder, there's not actually any work to do to commit @@ -173,8 +169,6 @@ private bool CommitOnTab() { CommitOnNonTypeChar(model.SelectedItem, model); } - - return true; } } } \ No newline at end of file From 34b7bcbc00457255fb64ae36ecfde612b67979ea Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 30 Nov 2016 00:03:24 -0800 Subject: [PATCH 19/19] Remove stale comment. --- .../IntelliSense/Completion/Controller_TabKey.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs index 7c21bad2d2e1a..44434b0d4d213 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs @@ -147,9 +147,6 @@ private bool QuestionMarkIsPrecededByIdentifierAndWhitespace( return current == questionPosition; } - /// - /// Returns whether or not the item was actually committed. - /// private void CommitOnTab(Action nextHandler) { AssertIsForeground();