Merge pull request dotnet#15572 from CyrusNajmabadi/completionBlocking

Allow languages to specify if they want completion to block or not.  

Fixes dotnet/fsharp#1825

src/EditorFeatures/Core/Implementation/IntelliSense/AbstractController.cs

@@ -26,7 +26,7 @@ internal abstract class AbstractController<TSession, TModel, TPresenterSession,
         // stop.
         protected TSession sessionOpt;
 
-        protected bool IsSessionActive { get { return sessionOpt != null; } }
+        protected bool IsSessionActive => sessionOpt != null;
 
         public AbstractController(ITextView textView, ITextBuffer subjectBuffer, IIntelliSensePresenter<TPresenterSession, TEditorSession> presenter, IAsynchronousOperationListener asyncListener, IDocumentProvider documentProvider, string asyncOperationId)
         {

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller.Session_Wait.cs

@@ -10,10 +10,23 @@ internal partial class Controller
     {
         internal partial class Session
         {
-            internal Model WaitForModel()
+            /// <summary>
+            /// Should only be called from <see cref="Controller.WaitForModel"/>.
+            /// </summary>
+            internal Model WaitForModel_DoNotCallDirectly(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);

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;
@@ -97,12 +95,43 @@ 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();
+            if (service == null || options == null)
+            {
+                return true;
+            }
+
+            return options.GetOption(CompletionOptions.BlockForCompletionItems, service.Language);
+        }
+
+        private Model WaitForModel()
+        {
+            this.AssertIsForeground();
+
+            var shouldBlock = ShouldBlockForCompletionItems();
+            var model = sessionOpt.WaitForModel_DoNotCallDirectly(shouldBlock);
+            if (model == null && !shouldBlock)
+            {
+                // 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();
+            }
+
+            return model;
+        }
+
         internal override void OnModelUpdated(Model modelOpt)
         {
             AssertIsForeground();
             if (modelOpt == null)
             {
-                this.StopModelComputation();
+                this.DismissSessionIfActive();
             }
             else
             {
@@ -230,13 +259,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;

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_AutomaticLineEnder.cs

@@ -29,7 +29,7 @@ void ICommandHandler<AutomaticLineEnderCommandArgs>.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();
             }
         }

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)

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;
             }
 

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(

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,15 +37,62 @@ void ICommandHandler<CommitUniqueCompletionListItemCommandArgs>.ExecuteCommand(
                 }
             }
 
-            // Get the selected item.  If it's unique, then we want to commit it.
-            var model = this.sessionOpt.WaitForModel();
+            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;
+            }
+
+            // 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)
             {
                 // Computation failed.  Just pass this command on.
                 nextHandler();
                 return;
             }
 
+            CommitIfUnique(model);
+        }
+
+        private void CommitUniqueCompletionListItemAsynchronously()
+        {
+            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<Model>)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)
+        {
+            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

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_CutPaste.cs

@@ -18,7 +18,7 @@ CommandState ICommandHandler<CutCommandArgs>.GetCommandState(CutCommandArgs args
         void ICommandHandler<CutCommandArgs>.ExecuteCommand(CutCommandArgs args, Action nextHandler)
         {
             AssertIsForeground();
-            EnsureCompletionSessionStopped();
+            DismissSessionIfActive();
             nextHandler();
         }
 
@@ -31,17 +31,8 @@ CommandState ICommandHandler<PasteCommandArgs>.GetCommandState(PasteCommandArgs
         void ICommandHandler<PasteCommandArgs>.ExecuteCommand(PasteCommandArgs args, Action nextHandler)
         {
             AssertIsForeground();
-            EnsureCompletionSessionStopped();
+            DismissSessionIfActive();
             nextHandler();
         }
-
-        private void EnsureCompletionSessionStopped()
-        {
-            AssertIsForeground();
-            if (this.sessionOpt != null)
-            {
-                StopModelComputation();
-            }
-        }
     }
 }

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_InvokeCompletionList.cs

@@ -21,10 +21,7 @@ void ICommandHandler<InvokeCompletionListCommandArgs>.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.

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
             {

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_ReturnKey.cs

@@ -27,11 +27,8 @@ void ICommandHandler<ReturnKeyCommandArgs>.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
@@ -48,7 +45,7 @@ private void CommitOnEnter(out bool sendThrough, 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)

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_SnippetCommands.cs

@@ -41,11 +41,7 @@ void ICommandHandler<SurroundWithCommandArgs>.ExecuteCommand(SurroundWithCommand
 
         private void DismissCompletionForSnippetPicker(Action nextHandler)
         {
-            if (sessionOpt != null)
-            {
-                StopModelComputation();
-            }
-
+            DismissSessionIfActive();
             nextHandler();
         }
     }

src/EditorFeatures/Core/Implementation/IntelliSense/Completion/Controller_TabKey.cs

@@ -37,19 +37,15 @@ void ICommandHandler<TabKeyCommandArgs>.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);
+            CommitOnTab(nextHandler);
 
-            // 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();
-            }
+            // After tab, we always want to be in an inactive state.
+            this.DismissSessionIfActive();
         }
 
         private bool TryInvokeSnippetCompletion(TabKeyCommandArgs args)
@@ -151,29 +147,25 @@ private bool QuestionMarkIsPrecededByIdentifierAndWhitespace(
             return current == questionPosition;
         }
 
-        private void CommitOnTab(out bool committed)
+        private void CommitOnTab(Action nextHandler)
         {
             AssertIsForeground();
 
-            var model = sessionOpt.WaitForModel();
+            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)
             {
-                committed = false;
+                nextHandler();
                 return;
             }
 
             // 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;
         }
     }
 }