diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs index edc8fb272a..7a87b25931 100644 --- a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs +++ b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs @@ -10,7 +10,7 @@ using System.Management.Automation.Language; using System.Threading; using System.Threading.Tasks; -namespace System.Management.Automation.Subsystem +namespace System.Management.Automation.Subsystem.Prediction { /// /// The class represents the prediction result from a predictor. @@ -59,9 +59,9 @@ namespace System.Management.Automation.Subsystem /// The object from parsing the current command line input. /// The objects from parsing the current command line input. /// A list of objects. - public static Task?> PredictInput(string client, Ast ast, Token[] astTokens) + public static Task?> PredictInputAsync(PredictionClient client, Ast ast, Token[] astTokens) { - return PredictInput(client, ast, astTokens, millisecondsTimeout: 20); + return PredictInputAsync(client, ast, astTokens, millisecondsTimeout: 20); } /// @@ -72,7 +72,7 @@ namespace System.Management.Automation.Subsystem /// The objects from parsing the current command line input. /// The milliseconds to timeout. /// A list of objects. - public static async Task?> PredictInput(string client, Ast ast, Token[] astTokens, int millisecondsTimeout) + public static async Task?> PredictInputAsync(PredictionClient client, Ast ast, Token[] astTokens, int millisecondsTimeout) { Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); @@ -86,17 +86,13 @@ namespace System.Management.Automation.Subsystem var tasks = new Task[predictors.Count]; using var cancellationSource = new CancellationTokenSource(); + Func callBack = GetCallBack(client, context, cancellationSource); + for (int i = 0; i < predictors.Count; i++) { ICommandPredictor predictor = predictors[i]; - tasks[i] = Task.Factory.StartNew( - state => - { - var predictor = (ICommandPredictor)state!; - SuggestionPackage pkg = predictor.GetSuggestion(client, context, cancellationSource.Token); - return pkg.SuggestionEntries?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, pkg.Session, pkg.SuggestionEntries) : null; - }, + callBack, predictor, cancellationSource.Token, TaskCreationOptions.DenyChildAttach, @@ -122,6 +118,21 @@ namespace System.Management.Automation.Subsystem } return resultList; + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no predictor is registered. + static Func GetCallBack( + PredictionClient client, + PredictionContext context, + CancellationTokenSource cancellationSource) + { + return state => + { + var predictor = (ICommandPredictor)state!; + SuggestionPackage pkg = predictor.GetSuggestion(client, context, cancellationSource.Token); + return pkg.SuggestionEntries?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, pkg.Session, pkg.SuggestionEntries) : null; + }; + } } /// @@ -129,7 +140,7 @@ namespace System.Management.Automation.Subsystem /// /// Represents the client that initiates the call. /// History command lines provided as references for prediction. - public static void OnCommandLineAccepted(string client, IReadOnlyList history) + public static void OnCommandLineAccepted(PredictionClient client, IReadOnlyList history) { Requires.NotNull(history, nameof(history)); @@ -139,16 +150,54 @@ namespace System.Management.Automation.Subsystem return; } + Action? callBack = null; foreach (ICommandPredictor predictor in predictors) { - if (predictor.SupportEarlyProcessing) + if (predictor.CanAcceptFeedback(client, PredictorFeedbackKind.CommandLineAccepted)) { - ThreadPool.QueueUserWorkItem( - state => state.StartEarlyProcessing(client, history), - predictor, - preferLocal: false); + callBack ??= GetCallBack(client, history); + ThreadPool.QueueUserWorkItem(callBack, predictor, preferLocal: false); } } + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no predictor is registered, or no registered predictor accepts this feedback. + static Action GetCallBack(PredictionClient client, IReadOnlyList history) + { + return predictor => predictor.OnCommandLineAccepted(client, history); + } + } + + /// + /// Allow registered predictors to know the execution result (success/failure) of the last accepted command line. + /// + /// Represents the client that initiates the call. + /// The last accepted command line. + /// Whether the execution of the last command line was successful. + public static void OnCommandLineExecuted(PredictionClient client, string commandLine, bool success) + { + var predictors = SubsystemManager.GetSubsystems(); + if (predictors.Count == 0) + { + return; + } + + Action? callBack = null; + foreach (ICommandPredictor predictor in predictors) + { + if (predictor.CanAcceptFeedback(client, PredictorFeedbackKind.CommandLineExecuted)) + { + callBack ??= GetCallBack(client, commandLine, success); + ThreadPool.QueueUserWorkItem(callBack, predictor, preferLocal: false); + } + } + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no predictor is registered, or no registered predictor accepts this feedback. + static Action GetCallBack(PredictionClient client, string commandLine, bool success) + { + return predictor => predictor.OnCommandLineExecuted(client, commandLine, success); + } } /// @@ -161,7 +210,7 @@ namespace System.Management.Automation.Subsystem /// When the value is greater than 0, it's the number of displayed suggestions from the list returned in , starting from the index 0. /// When the value is less than or equal to 0, it means a single suggestion from the list got displayed, and the index is the absolute value. /// - public static void OnSuggestionDisplayed(string client, Guid predictorId, uint session, int countOrIndex) + public static void OnSuggestionDisplayed(PredictionClient client, Guid predictorId, uint session, int countOrIndex) { var predictors = SubsystemManager.GetSubsystems(); if (predictors.Count == 0) @@ -171,14 +220,24 @@ namespace System.Management.Automation.Subsystem foreach (ICommandPredictor predictor in predictors) { - if (predictor.AcceptFeedback && predictor.Id == predictorId) + if (predictor.Id == predictorId) { - ThreadPool.QueueUserWorkItem( - state => state.OnSuggestionDisplayed(client, session, countOrIndex), - predictor, - preferLocal: false); + if (predictor.CanAcceptFeedback(client, PredictorFeedbackKind.SuggestionDisplayed)) + { + Action callBack = GetCallBack(client, session, countOrIndex); + ThreadPool.QueueUserWorkItem(callBack, predictor, preferLocal: false); + } + + break; } } + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no predictor is registered, or no registered predictor accepts this feedback. + static Action GetCallBack(PredictionClient client, uint session, int countOrIndex) + { + return predictor => predictor.OnSuggestionDisplayed(client, session, countOrIndex); + } } /// @@ -188,7 +247,7 @@ namespace System.Management.Automation.Subsystem /// The identifier of the predictor whose prediction result was accepted. /// The mini-session where the accepted suggestion came from. /// The accepted suggestion text. - public static void OnSuggestionAccepted(string client, Guid predictorId, uint session, string suggestionText) + public static void OnSuggestionAccepted(PredictionClient client, Guid predictorId, uint session, string suggestionText) { Requires.NotNullOrEmpty(suggestionText, nameof(suggestionText)); @@ -200,14 +259,24 @@ namespace System.Management.Automation.Subsystem foreach (ICommandPredictor predictor in predictors) { - if (predictor.AcceptFeedback && predictor.Id == predictorId) + if (predictor.Id == predictorId) { - ThreadPool.QueueUserWorkItem( - state => state.OnSuggestionAccepted(client, session, suggestionText), - predictor, - preferLocal: false); + if (predictor.CanAcceptFeedback(client, PredictorFeedbackKind.SuggestionAccepted)) + { + Action callBack = GetCallBack(client, session, suggestionText); + ThreadPool.QueueUserWorkItem(callBack, predictor, preferLocal: false); + } + + break; } } + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no predictor is registered, or no registered predictor accepts this feedback. + static Action GetCallBack(PredictionClient client, uint session, string suggestionText) + { + return predictor => predictor.OnSuggestionAccepted(client, session, suggestionText); + } } } } diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs index d55d37a90d..bd8a48a85d 100644 --- a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs +++ b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs @@ -9,7 +9,7 @@ using System.Management.Automation.Internal; using System.Management.Automation.Language; using System.Threading; -namespace System.Management.Automation.Subsystem +namespace System.Management.Automation.Subsystem.Prediction { /// /// Interface for implementing a predictor plugin. @@ -26,57 +26,132 @@ namespace System.Management.Automation.Subsystem /// SubsystemKind ISubsystem.Kind => SubsystemKind.CommandPredictor; - /// - /// Gets a value indicating whether the predictor supports early processing. - /// - bool SupportEarlyProcessing { get; } - - /// - /// Gets a value indicating whether the predictor accepts feedback about the previous suggestion. - /// - bool AcceptFeedback { get; } - - /// - /// A command line was accepted to execute. - /// The predictor can start processing early as needed with the latest history. - /// - /// Represents the client that initiates the call. - /// History command lines provided as references for prediction. - void StartEarlyProcessing(string clientId, IReadOnlyList history); - /// /// Get the predictive suggestions. It indicates the start of a suggestion rendering session. /// - /// Represents the client that initiates the call. + /// Represents the client that initiates the call. /// The object to be used for prediction. /// The cancellation token to cancel the prediction. /// An instance of . - SuggestionPackage GetSuggestion(string clientId, PredictionContext context, CancellationToken cancellationToken); + SuggestionPackage GetSuggestion(PredictionClient client, PredictionContext context, CancellationToken cancellationToken); + + /// + /// Gets a value indicating whether the predictor accepts a specific kind of feedback. + /// + /// Represents the client that initiates the call. + /// A specific type of feedback. + /// True or false, to indicate whether the specific feedback is accepted. + bool CanAcceptFeedback(PredictionClient client, PredictorFeedbackKind feedback); /// /// One or more suggestions provided by the predictor were displayed to the user. /// - /// Represents the client that initiates the call. + /// Represents the client that initiates the call. /// The mini-session where the displayed suggestions came from. /// /// When the value is greater than 0, it's the number of displayed suggestions from the list returned in , starting from the index 0. /// When the value is less than or equal to 0, it means a single suggestion from the list got displayed, and the index is the absolute value. /// - void OnSuggestionDisplayed(string clientId, uint session, int countOrIndex); + void OnSuggestionDisplayed(PredictionClient client, uint session, int countOrIndex); /// /// The suggestion provided by the predictor was accepted. /// - /// Represents the client that initiates the call. + /// Represents the client that initiates the call. /// Represents the mini-session where the accepted suggestion came from. /// The accepted suggestion text. - void OnSuggestionAccepted(string clientId, uint session, string acceptedSuggestion); + void OnSuggestionAccepted(PredictionClient client, uint session, string acceptedSuggestion); + + /// + /// A command line was accepted to execute. + /// The predictor can start processing early as needed with the latest history. + /// + /// Represents the client that initiates the call. + /// History command lines provided as references for prediction. + void OnCommandLineAccepted(PredictionClient client, IReadOnlyList history); + + /// + /// A command line was done execution. + /// + /// Represents the client that initiates the call. + /// The last accepted command line. + /// Shows whether the execution was successful. + void OnCommandLineExecuted(PredictionClient client, string commandLine, bool success); + } + + /// + /// Kinds of feedback a predictor can choose to accept. + /// + public enum PredictorFeedbackKind + { + /// + /// Feedback when one or more suggestions are displayed to the user. + /// + SuggestionDisplayed, + + /// + /// Feedback when a suggestion is accepted by the user. + /// + SuggestionAccepted, + + /// + /// Feedback when a command line is accepted by the user. + /// + CommandLineAccepted, + + /// + /// Feedback when the accepted command line finishes its execution. + /// + CommandLineExecuted, + } + + /// + /// Kinds of prediction clients. + /// + public enum PredictionClientKind + { + /// + /// A terminal client, representing the command-line experience. + /// + Terminal, + + /// + /// An editor client, representing the editor experience. + /// + Editor, + } + + /// + /// The class represents a client that interacts with predictors. + /// + public sealed class PredictionClient + { + /// + /// Gets the client name. + /// + public string Name { get; } + + /// + /// Gets the client kind. + /// + public PredictionClientKind Kind { get; } + + /// + /// Initializes a new instance of the class. + /// + /// Name of the interactive client. + /// Kind of the interactive client. + public PredictionClient(string name, PredictionClientKind kind) + { + Name = name; + Kind = kind; + } } /// /// Context information about the user input. /// - public class PredictionContext + public sealed class PredictionContext { /// /// Gets the abstract syntax tree (AST) generated from parsing the user input. diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index 81a27f6127..1d90fc7334 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; using System.Management.Automation.Internal; +using System.Management.Automation.Subsystem.Prediction; namespace System.Management.Automation.Subsystem { diff --git a/test/xUnit/csharp/test_Prediction.cs b/test/xUnit/csharp/test_Prediction.cs index 8f018ebb9e..12e61dcd84 100644 --- a/test/xUnit/csharp/test_Prediction.cs +++ b/test/xUnit/csharp/test_Prediction.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Management.Automation.Language; using System.Management.Automation.Subsystem; +using System.Management.Automation.Subsystem.Prediction; using System.Threading; using Xunit; @@ -18,6 +19,8 @@ namespace PSTests.Sequential public List History { get; } + public List Results { get; } + public List AcceptedSuggestions { get; } public List DisplayedSuggestions { get; } @@ -47,39 +50,30 @@ namespace PSTests.Sequential _delay = delay; History = new List(); + Results = new List(); AcceptedSuggestions = new List(); DisplayedSuggestions = new List(); } + public void Clear() + { + History.Clear(); + Results.Clear(); + AcceptedSuggestions.Clear(); + DisplayedSuggestions.Clear(); + } + + #region "Interface implementation" + public Guid Id => _id; public string Name => _name; public string Description => _description; - bool ICommandPredictor.SupportEarlyProcessing => true; + bool ICommandPredictor.CanAcceptFeedback(PredictionClient client, PredictorFeedbackKind feedback) => true; - bool ICommandPredictor.AcceptFeedback => true; - - public void StartEarlyProcessing(string clientId, IReadOnlyList history) - { - foreach (string item in history) - { - History.Add($"{clientId}-{item}"); - } - } - - public void OnSuggestionDisplayed(string clientId, uint session, int countOrIndex) - { - DisplayedSuggestions.Add($"{clientId}-{session}-{countOrIndex}"); - } - - public void OnSuggestionAccepted(string clientId, uint session, string acceptedSuggestion) - { - AcceptedSuggestions.Add($"{clientId}-{session}-{acceptedSuggestion}"); - } - - public SuggestionPackage GetSuggestion(string clientId, PredictionContext context, CancellationToken cancellationToken) + public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContext context, CancellationToken cancellationToken) { if (_delay) { @@ -92,18 +86,44 @@ namespace PSTests.Sequential var userInput = context.InputAst.Extent.Text; var entries = new List { - new PredictiveSuggestion($"'{userInput}' from '{clientId}' - TEST-1 from {Name}"), - new PredictiveSuggestion($"'{userInput}' from '{clientId}' - TeSt-2 from {Name}"), + new PredictiveSuggestion($"'{userInput}' from '{client.Name}' - TEST-1 from {Name}"), + new PredictiveSuggestion($"'{userInput}' from '{client.Name}' - TeSt-2 from {Name}"), }; return new SuggestionPackage(56, entries); } + + public void OnSuggestionDisplayed(PredictionClient client, uint session, int countOrIndex) + { + DisplayedSuggestions.Add($"{client.Name}-{session}-{countOrIndex}"); + } + + public void OnSuggestionAccepted(PredictionClient client, uint session, string acceptedSuggestion) + { + AcceptedSuggestions.Add($"{client.Name}-{session}-{acceptedSuggestion}"); + } + + public void OnCommandLineAccepted(PredictionClient client, IReadOnlyList history) + { + foreach (string item in history) + { + History.Add($"{client.Name}-{item}"); + } + } + + public void OnCommandLineExecuted(PredictionClient client, string commandLine, bool success) + { + Results.Add($"{client.Name}-{commandLine}-{success}"); + } + + #endregion } public static class CommandPredictionTests { private const string Client = "PredictionTest"; private const uint Session = 56; + private static PredictionClient predClient = new(Client, PredictionClientKind.Terminal); [Fact] public static void PredictInput() @@ -114,7 +134,7 @@ namespace PSTests.Sequential Ast ast = Parser.ParseInput(Input, out Token[] tokens, out _); // Returns null when no predictor implementation registered - List results = CommandPrediction.PredictInput(Client, ast, tokens).Result; + List results = CommandPrediction.PredictInputAsync(predClient, ast, tokens).Result; Assert.Null(results); try @@ -127,7 +147,7 @@ namespace PSTests.Sequential // cannot finish before the specified timeout. // The specified timeout is exaggerated to make the test reliable. // xUnit must spin up a lot tasks, which makes the test unreliable when the time difference between 'delay' and 'timeout' is small. - results = CommandPrediction.PredictInput(Client, ast, tokens, millisecondsTimeout: 1000).Result; + results = CommandPrediction.PredictInputAsync(predClient, ast, tokens, millisecondsTimeout: 1000).Result; Assert.Single(results); PredictionResult res = results[0]; @@ -140,7 +160,7 @@ namespace PSTests.Sequential // Expect the results from both 'slow' and 'fast' predictors // Same here -- the specified timeout is exaggerated to make the test reliable. // xUnit must spin up a lot tasks, which makes the test unreliable when the time difference between 'delay' and 'timeout' is small. - results = CommandPrediction.PredictInput(Client, ast, tokens, millisecondsTimeout: 4000).Result; + results = CommandPrediction.PredictInputAsync(predClient, ast, tokens, millisecondsTimeout: 4000).Result; Assert.Equal(2, results.Count); PredictionResult res1 = results[0]; @@ -170,6 +190,9 @@ namespace PSTests.Sequential MyPredictor slow = MyPredictor.SlowPredictor; MyPredictor fast = MyPredictor.FastPredictor; + slow.Clear(); + fast.Clear(); + try { // Register 2 predictor implementations @@ -179,16 +202,19 @@ namespace PSTests.Sequential var history = new[] { "hello", "world" }; var ids = new HashSet { slow.Id, fast.Id }; - CommandPrediction.OnCommandLineAccepted(Client, history); - CommandPrediction.OnSuggestionDisplayed(Client, slow.Id, Session, 2); - CommandPrediction.OnSuggestionDisplayed(Client, fast.Id, Session, -1); - CommandPrediction.OnSuggestionAccepted(Client, slow.Id, Session, "Yeah"); + CommandPrediction.OnCommandLineAccepted(predClient, history); + CommandPrediction.OnCommandLineExecuted(predClient, "last_input", true); + CommandPrediction.OnSuggestionDisplayed(predClient, slow.Id, Session, 2); + CommandPrediction.OnSuggestionDisplayed(predClient, fast.Id, Session, -1); + CommandPrediction.OnSuggestionAccepted(predClient, slow.Id, Session, "Yeah"); - // The calls to 'StartEarlyProcessing' and 'OnSuggestionAccepted' are queued in thread pool, - // so we wait a bit to make sure the calls are done. - while (slow.History.Count == 0 || slow.AcceptedSuggestions.Count == 0) + // The feedback calls are queued in thread pool, so let's wait a bit to make sure the calls are done. + while (slow.History.Count == 0 || fast.History.Count == 0 || + slow.Results.Count == 0 || fast.Results.Count == 0 || + slow.DisplayedSuggestions.Count == 0 || fast.DisplayedSuggestions.Count == 0 || + slow.AcceptedSuggestions.Count == 0) { - Thread.Sleep(10); + Thread.Sleep(100); } Assert.Equal(2, slow.History.Count); @@ -199,6 +225,12 @@ namespace PSTests.Sequential Assert.Equal($"{Client}-{history[0]}", fast.History[0]); Assert.Equal($"{Client}-{history[1]}", fast.History[1]); + Assert.Single(slow.Results); + Assert.Equal($"{Client}-last_input-True", slow.Results[0]); + + Assert.Single(fast.Results); + Assert.Equal($"{Client}-last_input-True", fast.Results[0]); + Assert.Single(slow.DisplayedSuggestions); Assert.Equal($"{Client}-{Session}-2", slow.DisplayedSuggestions[0]); diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index f2ca308221..54fdcbf83b 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -4,6 +4,7 @@ using System; using System.Collections.ObjectModel; using System.Management.Automation.Subsystem; +using System.Management.Automation.Subsystem.Prediction; using System.Threading; using Xunit; @@ -97,8 +98,9 @@ namespace PSTests.Sequential const string Client = "SubsystemTest"; const string Input = "Hello world"; + var predClient = new PredictionClient(Client, PredictionClientKind.Terminal); var predCxt = PredictionContext.Create(Input); - var results = impl.GetSuggestion(Client, predCxt, CancellationToken.None); + var results = impl.GetSuggestion(predClient, predCxt, CancellationToken.None); Assert.Equal($"'{Input}' from '{Client}' - TEST-1 from {impl.Name}", results.SuggestionEntries[0].SuggestionText); Assert.Equal($"'{Input}' from '{Client}' - TeSt-2 from {impl.Name}", results.SuggestionEntries[1].SuggestionText);