diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs index 30deb1b3093..edc8fb272a2 100644 --- a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs +++ b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs @@ -27,15 +27,22 @@ public sealed class PredictionResult /// public string Name { get; } + /// + /// Gets the mini-session id that represents a specific invocation to the API of the predictor. + /// When it's not specified, it's considered by a client that the predictor doesn't expect feedback. + /// + public uint? Session { get; } + /// /// Gets the suggestions. /// public IReadOnlyList Suggestions { get; } - internal PredictionResult(Guid id, string name, List suggestions) + internal PredictionResult(Guid id, string name, uint? session, List suggestions) { Id = id; Name = name; + Session = session; Suggestions = suggestions; } } @@ -48,22 +55,24 @@ public static class CommandPrediction /// /// Collect the predictive suggestions from registered predictors using the default timeout. /// + /// Represents the client that initiates the call. /// 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(Ast ast, Token[] astTokens) + public static Task?> PredictInput(string client, Ast ast, Token[] astTokens) { - return PredictInput(ast, astTokens, millisecondsTimeout: 20); + return PredictInput(client, ast, astTokens, millisecondsTimeout: 20); } /// /// Collect the predictive suggestions from registered predictors using the specified timeout. /// + /// Represents the client that initiates the call. /// The object from parsing the current command line input. /// The objects from parsing the current command line input. /// The milliseconds to timeout. /// A list of objects. - public static async Task?> PredictInput(Ast ast, Token[] astTokens, int millisecondsTimeout) + public static async Task?> PredictInput(string client, Ast ast, Token[] astTokens, int millisecondsTimeout) { Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); @@ -85,8 +94,8 @@ public static class CommandPrediction state => { var predictor = (ICommandPredictor)state!; - List? texts = predictor.GetSuggestion(context, cancellationSource.Token); - return texts?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, texts) : null; + SuggestionPackage pkg = predictor.GetSuggestion(client, context, cancellationSource.Token); + return pkg.SuggestionEntries?.Count > 0 ? new PredictionResult(predictor.Id, predictor.Name, pkg.Session, pkg.SuggestionEntries) : null; }, predictor, cancellationSource.Token, @@ -99,7 +108,7 @@ await Task.WhenAny( Task.Delay(millisecondsTimeout, cancellationSource.Token)).ConfigureAwait(false); cancellationSource.Cancel(); - var results = new List(predictors.Count); + var resultList = new List(predictors.Count); foreach (Task task in tasks) { if (task.IsCompletedSuccessfully) @@ -107,19 +116,20 @@ await Task.WhenAny( PredictionResult? result = task.Result; if (result != null) { - results.Add(result); + resultList.Add(result); } } } - return results; + return resultList; } /// /// Allow registered predictors to do early processing when a command line is accepted. /// + /// Represents the client that initiates the call. /// History command lines provided as references for prediction. - public static void OnCommandLineAccepted(IReadOnlyList history) + public static void OnCommandLineAccepted(string client, IReadOnlyList history) { Requires.NotNull(history, nameof(history)); @@ -134,7 +144,37 @@ public static void OnCommandLineAccepted(IReadOnlyList history) if (predictor.SupportEarlyProcessing) { ThreadPool.QueueUserWorkItem( - state => state.StartEarlyProcessing(history), + state => state.StartEarlyProcessing(client, history), + predictor, + preferLocal: false); + } + } + } + + /// + /// Send feedback to a predictor when one or more suggestions from it were displayed to the user. + /// + /// Represents the client that initiates the call. + /// The identifier of the predictor whose prediction result was accepted. + /// 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. + /// + public static void OnSuggestionDisplayed(string client, Guid predictorId, uint session, int countOrIndex) + { + var predictors = SubsystemManager.GetSubsystems(); + if (predictors.Count == 0) + { + return; + } + + foreach (ICommandPredictor predictor in predictors) + { + if (predictor.AcceptFeedback && predictor.Id == predictorId) + { + ThreadPool.QueueUserWorkItem( + state => state.OnSuggestionDisplayed(client, session, countOrIndex), predictor, preferLocal: false); } @@ -142,11 +182,13 @@ public static void OnCommandLineAccepted(IReadOnlyList history) } /// - /// Send feedback to predictors about their last suggestions. + /// Send feedback to a predictor when a suggestion from it was accepted. /// + /// Represents the client that initiates the call. /// 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(Guid predictorId, string suggestionText) + public static void OnSuggestionAccepted(string client, Guid predictorId, uint session, string suggestionText) { Requires.NotNullOrEmpty(suggestionText, nameof(suggestionText)); @@ -161,7 +203,7 @@ public static void OnSuggestionAccepted(Guid predictorId, string suggestionText) if (predictor.AcceptFeedback && predictor.Id == predictorId) { ThreadPool.QueueUserWorkItem( - state => state.OnSuggestionAccepted(suggestionText), + state => state.OnSuggestionAccepted(client, session, suggestionText), predictor, preferLocal: false); } diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs index 0edd768cf89..d55d37a90df 100644 --- a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs +++ b/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs @@ -40,22 +40,37 @@ public interface ICommandPredictor : ISubsystem /// 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(IReadOnlyList history); + void StartEarlyProcessing(string clientId, IReadOnlyList history); /// - /// The suggestion given by the predictor was accepted. + /// Get the predictive suggestions. It indicates the start of a suggestion rendering session. /// - /// The accepted suggestion text. - void OnSuggestionAccepted(string acceptedSuggestion); + /// 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); /// - /// Get the predictive suggestions. + /// One or more suggestions provided by the predictor were displayed to the user. /// - /// The object to be used for prediction. - /// The cancellation token to cancel the prediction. - /// A list of predictive suggestions. - List? GetSuggestion(PredictionContext context, CancellationToken cancellationToken); + /// 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); + + /// + /// The suggestion provided by the predictor was accepted. + /// + /// 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); } /// @@ -160,4 +175,47 @@ public PredictiveSuggestion(string suggestion, string? toolTip) ToolTip = toolTip; } } + + /// + /// A package returned from . + /// + public struct SuggestionPackage + { + /// + /// Gets the mini-session that represents a specific invocation to . + /// When it's not specified, it's considered by a client that the predictor doesn't expect feedback. + /// + public uint? Session { get; } + + /// + /// Gets the suggestion entries returned from that mini-session. + /// + public List? SuggestionEntries { get; } + + /// + /// Initializes a new instance of the struct without providing a session id. + /// Note that, when a session id is not specified, it's considered by a client that the predictor doesn't expect feedback. + /// + /// The suggestions to return. + public SuggestionPackage(List suggestionEntries) + { + Requires.NotNullOrEmpty(suggestionEntries, nameof(suggestionEntries)); + + Session = null; + SuggestionEntries = suggestionEntries; + } + + /// + /// Initializes a new instance of the struct with the mini-session id and the suggestions. + /// + /// The mini-session where suggestions came from. + /// The suggestions to return. + public SuggestionPackage(uint session, List suggestionEntries) + { + Requires.NotNullOrEmpty(suggestionEntries, nameof(suggestionEntries)); + + Session = session; + SuggestionEntries = suggestionEntries; + } + } } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 6a4a863d23c..fa1ef13572e 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2312,6 +2312,14 @@ internal static void NotNullOrEmpty(string value, string paramName) } } + internal static void NotNullOrEmpty(ICollection value, string paramName) + { + if (value == null || value.Count == 0) + { + throw new ArgumentNullException(paramName); + } + } + internal static void Condition([DoesNotReturnIf(false)] bool precondition, string paramName) { if (!precondition) diff --git a/test/xUnit/csharp/test_Prediction.cs b/test/xUnit/csharp/test_Prediction.cs index 4d67c850f6d..8f018ebb9e1 100644 --- a/test/xUnit/csharp/test_Prediction.cs +++ b/test/xUnit/csharp/test_Prediction.cs @@ -20,9 +20,9 @@ public class MyPredictor : ICommandPredictor public List AcceptedSuggestions { get; } - public static readonly MyPredictor SlowPredictor; + public List DisplayedSuggestions { get; } - public static readonly MyPredictor FastPredictor; + public static readonly MyPredictor SlowPredictor, FastPredictor; static MyPredictor() { @@ -48,6 +48,7 @@ private MyPredictor(Guid id, string name, string description, bool delay) History = new List(); AcceptedSuggestions = new List(); + DisplayedSuggestions = new List(); } public Guid Id => _id; @@ -60,17 +61,25 @@ private MyPredictor(Guid id, string name, string description, bool delay) bool ICommandPredictor.AcceptFeedback => true; - public void StartEarlyProcessing(IReadOnlyList history) + public void StartEarlyProcessing(string clientId, IReadOnlyList history) { - History.AddRange(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 acceptedSuggestion) + public void OnSuggestionAccepted(string clientId, uint session, string acceptedSuggestion) { - AcceptedSuggestions.Add(acceptedSuggestion); + AcceptedSuggestions.Add($"{clientId}-{session}-{acceptedSuggestion}"); } - public List GetSuggestion(PredictionContext context, CancellationToken cancellationToken) + public SuggestionPackage GetSuggestion(string clientId, PredictionContext context, CancellationToken cancellationToken) { if (_delay) { @@ -81,24 +90,31 @@ public List GetSuggestion(PredictionContext context, Cance // You can get the user input from the AST. var userInput = context.InputAst.Extent.Text; - return new List { - new PredictiveSuggestion($"{userInput} TEST-1 from {Name}"), - new PredictiveSuggestion($"{userInput} TeSt-2 from {Name}"), + var entries = new List + { + new PredictiveSuggestion($"'{userInput}' from '{clientId}' - TEST-1 from {Name}"), + new PredictiveSuggestion($"'{userInput}' from '{clientId}' - TeSt-2 from {Name}"), }; + + return new SuggestionPackage(56, entries); } } public static class CommandPredictionTests { + private const string Client = "PredictionTest"; + private const uint Session = 56; + [Fact] public static void PredictInput() { + const string Input = "Hello world"; MyPredictor slow = MyPredictor.SlowPredictor; MyPredictor fast = MyPredictor.FastPredictor; - Ast ast = Parser.ParseInput("Hello world", out Token[] tokens, out _); + Ast ast = Parser.ParseInput(Input, out Token[] tokens, out _); // Returns null when no predictor implementation registered - List results = CommandPrediction.PredictInput(ast, tokens).Result; + List results = CommandPrediction.PredictInput(Client, ast, tokens).Result; Assert.Null(results); try @@ -111,32 +127,35 @@ public static void PredictInput() // 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(ast, tokens, millisecondsTimeout: 1000).Result; + results = CommandPrediction.PredictInput(Client, ast, tokens, millisecondsTimeout: 1000).Result; Assert.Single(results); PredictionResult res = results[0]; Assert.Equal(fast.Id, res.Id); + Assert.Equal(Session, res.Session); Assert.Equal(2, res.Suggestions.Count); - Assert.Equal($"Hello world TEST-1 from {fast.Name}", res.Suggestions[0].SuggestionText); - Assert.Equal($"Hello world TeSt-2 from {fast.Name}", res.Suggestions[1].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TEST-1 from {fast.Name}", res.Suggestions[0].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TeSt-2 from {fast.Name}", res.Suggestions[1].SuggestionText); // 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(ast, tokens, millisecondsTimeout: 4000).Result; + results = CommandPrediction.PredictInput(Client, ast, tokens, millisecondsTimeout: 4000).Result; Assert.Equal(2, results.Count); PredictionResult res1 = results[0]; Assert.Equal(slow.Id, res1.Id); + Assert.Equal(Session, res1.Session); Assert.Equal(2, res1.Suggestions.Count); - Assert.Equal($"Hello world TEST-1 from {slow.Name}", res1.Suggestions[0].SuggestionText); - Assert.Equal($"Hello world TeSt-2 from {slow.Name}", res1.Suggestions[1].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TEST-1 from {slow.Name}", res1.Suggestions[0].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TeSt-2 from {slow.Name}", res1.Suggestions[1].SuggestionText); PredictionResult res2 = results[1]; Assert.Equal(fast.Id, res2.Id); + Assert.Equal(Session, res2.Session); Assert.Equal(2, res2.Suggestions.Count); - Assert.Equal($"Hello world TEST-1 from {fast.Name}", res2.Suggestions[0].SuggestionText); - Assert.Equal($"Hello world TeSt-2 from {fast.Name}", res2.Suggestions[1].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TEST-1 from {fast.Name}", res2.Suggestions[0].SuggestionText); + Assert.Equal($"'{Input}' from '{Client}' - TeSt-2 from {fast.Name}", res2.Suggestions[1].SuggestionText); } finally { @@ -160,8 +179,10 @@ public static void Feedback() var history = new[] { "hello", "world" }; var ids = new HashSet { slow.Id, fast.Id }; - CommandPrediction.OnCommandLineAccepted(history); - CommandPrediction.OnSuggestionAccepted(slow.Id, "Yeah"); + 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"); // The calls to 'StartEarlyProcessing' and 'OnSuggestionAccepted' are queued in thread pool, // so we wait a bit to make sure the calls are done. @@ -171,15 +192,21 @@ public static void Feedback() } Assert.Equal(2, slow.History.Count); - Assert.Equal(history[0], slow.History[0]); - Assert.Equal(history[1], slow.History[1]); + Assert.Equal($"{Client}-{history[0]}", slow.History[0]); + Assert.Equal($"{Client}-{history[1]}", slow.History[1]); Assert.Equal(2, fast.History.Count); - Assert.Equal(history[0], fast.History[0]); - Assert.Equal(history[1], fast.History[1]); + Assert.Equal($"{Client}-{history[0]}", fast.History[0]); + Assert.Equal($"{Client}-{history[1]}", fast.History[1]); + + Assert.Single(slow.DisplayedSuggestions); + Assert.Equal($"{Client}-{Session}-2", slow.DisplayedSuggestions[0]); + + Assert.Single(fast.DisplayedSuggestions); + Assert.Equal($"{Client}-{Session}--1", fast.DisplayedSuggestions[0]); Assert.Single(slow.AcceptedSuggestions); - Assert.Equal("Yeah", slow.AcceptedSuggestions[0]); + Assert.Equal($"{Client}-{Session}-Yeah", slow.AcceptedSuggestions[0]); Assert.Empty(fast.AcceptedSuggestions); } diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index a45ebc116b5..f2ca308221f 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -95,10 +95,12 @@ public static void RegisterSubsystem() Assert.Null(impl.FunctionsToDefine); Assert.Equal(SubsystemKind.CommandPredictor, impl.Kind); - var predCxt = PredictionContext.Create("Hello world"); - var results = impl.GetSuggestion(predCxt, CancellationToken.None); - Assert.Equal($"Hello world TEST-1 from {impl.Name}", results[0].SuggestionText); - Assert.Equal($"Hello world TeSt-2 from {impl.Name}", results[1].SuggestionText); + const string Client = "SubsystemTest"; + const string Input = "Hello world"; + var predCxt = PredictionContext.Create(Input); + var results = impl.GetSuggestion(Client, 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); // Now validate the all-subsystem-implementation collection. ReadOnlyCollection impls = SubsystemManager.GetSubsystems(); diff --git a/tools/packaging/packaging.psm1 b/tools/packaging/packaging.psm1 index aaaee8688d0..548f724caec 100644 --- a/tools/packaging/packaging.psm1 +++ b/tools/packaging/packaging.psm1 @@ -2036,24 +2036,27 @@ function CopyReferenceAssemblies switch ($assemblyName) { { $_ -in $supportedRefList } { $refDll = Join-Path -Path $refBinPath -ChildPath "$assemblyName.dll" - Copy-Item $refDll -Destination $refNugetPath -Force - Write-Log "Copied file $refDll to $refNugetPath" + $refDoc = Join-Path -Path $refBinPath -ChildPath "$assemblyName.xml" + Copy-Item $refDll, $refDoc -Destination $refNugetPath -Force + Write-Log "Copied file '$refDll' and '$refDoc' to '$refNugetPath'" } "Microsoft.PowerShell.SDK" { foreach ($asmFileName in $assemblyFileList) { $refFile = Join-Path -Path $refBinPath -ChildPath $asmFileName if (Test-Path -Path $refFile) { - Copy-Item $refFile -Destination $refNugetPath -Force - Write-Log "Copied file $refFile to $refNugetPath" + $refDoc = Join-Path -Path $refBinPath -ChildPath ([System.IO.Path]::ChangeExtension($asmFileName, "xml")) + Copy-Item $refFile, $refDoc -Destination $refNugetPath -Force + Write-Log "Copied file '$refFile' and '$refDoc' to '$refNugetPath'" } } } default { $ref_SMA = Join-Path -Path $refBinPath -ChildPath System.Management.Automation.dll - Copy-Item $ref_SMA -Destination $refNugetPath -Force - Write-Log "Copied file $ref_SMA to $refNugetPath" + $ref_doc = Join-Path -Path $refBinPath -ChildPath System.Management.Automation.xml + Copy-Item $ref_SMA, $ref_doc -Destination $refNugetPath -Force + Write-Log "Copied file '$ref_SMA' and '$ref_doc' to '$refNugetPath'" } } } @@ -2235,8 +2238,13 @@ function New-ReferenceAssembly throw "$assemblyName.dll was not found at: $Linux64BinPath" } + $dllXmlDoc = Join-Path $Linux64BinPath "$assemblyName.xml" + if (-not (Test-Path $dllXmlDoc)) { + throw "$assemblyName.xml was not found at: $Linux64BinPath" + } + $genAPIArgs = "$linuxDllPath","-libPath:$Linux64BinPath,$Linux64BinPath\ref" - Write-Log "GenAPI cmd: $genAPIExe $genAPIArgsString" + Write-Log "GenAPI cmd: $genAPIExe $genAPIArgs" Start-NativeExecution { & $genAPIExe $genAPIArgs } | Out-File $generatedSource -Force Write-Log "Reference assembly file generated at: $generatedSource" @@ -2268,6 +2276,9 @@ function New-ReferenceAssembly Copy-Item $refBinPath $RefAssemblyDestinationPath -Force Write-Log "Reference assembly '$assemblyName.dll' built and copied to $RefAssemblyDestinationPath" + Copy-Item $dllXmlDoc $RefAssemblyDestinationPath -Force + Write-Log "Xml document '$assemblyName.xml' copied to $RefAssemblyDestinationPath" + if ($assemblyName -eq "System.Management.Automation") { $SMAReferenceAssembly = $refBinPath } @@ -2367,6 +2378,11 @@ function CleanupGeneratedSourceCode Pattern = "[System.Runtime.CompilerServices.CompilerGeneratedAttribute, System.Runtime.CompilerServices.NullableContextAttribute((byte)2)]" Replacement = "/* [System.Runtime.CompilerServices.CompilerGeneratedAttribute, System.Runtime.CompilerServices.NullableContextAttribute((byte)2)] */ " }, + @{ + ApplyTo = @("System.Management.Automation") + Pattern = "[System.Runtime.CompilerServices.CompilerGeneratedAttribute, System.Runtime.CompilerServices.IsReadOnlyAttribute]" + Replacement = "/* [System.Runtime.CompilerServices.CompilerGeneratedAttribute, System.Runtime.CompilerServices.IsReadOnlyAttribute] */ " + }, @{ ApplyTo = @("System.Management.Automation", "Microsoft.PowerShell.ConsoleHost") Pattern = "[System.Runtime.CompilerServices.NullableAttribute((byte)2)]"