From 6e343da08ba8e7e8470ffe3ec74a90989a2840f6 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 22 Sep 2022 11:06:47 -0700 Subject: [PATCH 01/29] Clean up unused code regarding suggestion --- .../host/msh/ConsoleHost.cs | 1 - .../engine/hostifaces/HostUtilities.cs | 84 ------------------- .../resources/SuggestionStrings.resx | 6 -- 3 files changed, 91 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 812f42cfd5e..cb1688d4aa0 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2489,7 +2489,6 @@ internal void Run(bool inputLoopIsNested) if (inBlockMode) { // use a special prompt that denotes block mode - prompt = ">> "; } else diff --git a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index b3e29423a83..3fe4f472e1b 100644 --- a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs +++ b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs @@ -79,20 +79,6 @@ private static List InitializeSuggestions() var suggestions = new List( new Hashtable[] { - NewSuggestion( - id: 1, - category: "Transactions", - matchType: SuggestionMatchType.Command, - rule: "^Start-Transaction", - suggestion: SuggestionStrings.Suggestion_StartTransaction, - enabled: true), - NewSuggestion( - id: 2, - category: "Transactions", - matchType: SuggestionMatchType.Command, - rule: "^Use-Transaction", - suggestion: SuggestionStrings.Suggestion_UseTransaction, - enabled: true), NewSuggestion( id: 3, category: "General", @@ -561,30 +547,6 @@ internal static string RemoveIdentifierInfoFromMessage(string message, out bool return message; } - /// - /// Create suggestion with string rule and suggestion. - /// - /// Identifier for the suggestion. - /// Category for the suggestion. - /// Suggestion match type. - /// Rule to match. - /// Suggestion to return. - /// True if the suggestion is enabled. - /// Hashtable representing the suggestion. - private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, string rule, string suggestion, bool enabled) - { - Hashtable result = new Hashtable(StringComparer.CurrentCultureIgnoreCase); - - result["Id"] = id; - result["Category"] = category; - result["MatchType"] = matchType; - result["Rule"] = rule; - result["Suggestion"] = suggestion; - result["Enabled"] = enabled; - - return result; - } - /// /// Create suggestion with string rule and scriptblock suggestion. /// @@ -639,15 +601,6 @@ private static Hashtable NewSuggestion(int id, string category, SuggestionMatchT return result; } - /// - /// Get suggestion text from suggestion scriptblock. - /// - [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "Need to keep this for legacy reflection based use")] - private static string GetSuggestionText(object suggestion, PSModuleInfo invocationModule) - { - return GetSuggestionText(suggestion, null, invocationModule); - } - /// /// Get suggestion text from suggestion scriptblock with arguments. /// @@ -702,43 +655,6 @@ runspace.ConnectionInfo is VMConnectionInfo || return string.Format(CultureInfo.InvariantCulture, "[{0}]: {1}", runspace.ConnectionInfo.ComputerName, basePrompt); } - internal static bool IsProcessInteractive(InvocationInfo invocationInfo) - { -#if CORECLR - return false; -#else - // CommandOrigin != Runspace means it is in a script - if (invocationInfo.CommandOrigin != CommandOrigin.Runspace) - return false; - - // If we don't own the window handle, we've been invoked - // from another process that just calls "PowerShell -Command" - if (System.Diagnostics.Process.GetCurrentProcess().MainWindowHandle == IntPtr.Zero) - return false; - - // If the window has been idle for less than two seconds, - // they're probably still calling "PowerShell -Command" - // but from Start-Process, or the StartProcess API - try - { - System.Diagnostics.Process currentProcess = System.Diagnostics.Process.GetCurrentProcess(); - TimeSpan timeSinceStart = DateTime.Now - currentProcess.StartTime; - TimeSpan idleTime = timeSinceStart - currentProcess.TotalProcessorTime; - - // Making it 2 seconds because of things like delayed prompt - if (idleTime.TotalSeconds > 2) - return true; - } - catch (System.ComponentModel.Win32Exception) - { - // Don't have access to the properties - return false; - } - - return false; -#endif - } - /// /// Create a configured remote runspace from provided name. /// diff --git a/src/System.Management.Automation/resources/SuggestionStrings.resx b/src/System.Management.Automation/resources/SuggestionStrings.resx index cd44c33e759..7a8dabf2958 100644 --- a/src/System.Management.Automation/resources/SuggestionStrings.resx +++ b/src/System.Management.Automation/resources/SuggestionStrings.resx @@ -117,12 +117,6 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - Once a transaction is started, only commands that get called with the -UseTransaction flag become part of that transaction. - - - The Use-Transaction cmdlet is intended for scripting of transaction-enabled .NET objects. Its ScriptBlock should contain nothing else. - The command {0} was not found, but does exist in the current location. PowerShell does not load commands from the current location by default. If you trust this command, instead type: "{1}". See "get-help about_Command_Precedence" for more details. From 83a8ec8daf78d258a68faedf09fd5ecfd9ccdca0 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 22 Sep 2022 11:20:15 -0700 Subject: [PATCH 02/29] Unify the folder name for existing subsystems --- .../CommandPrediction.cs | 0 .../ICommandPredictor.cs | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/System.Management.Automation/engine/Subsystem/{CommandPrediction => PredictionSubsystem}/CommandPrediction.cs (100%) rename src/System.Management.Automation/engine/Subsystem/{CommandPrediction => PredictionSubsystem}/ICommandPredictor.cs (100%) diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs b/src/System.Management.Automation/engine/Subsystem/PredictionSubsystem/CommandPrediction.cs similarity index 100% rename from src/System.Management.Automation/engine/Subsystem/CommandPrediction/CommandPrediction.cs rename to src/System.Management.Automation/engine/Subsystem/PredictionSubsystem/CommandPrediction.cs diff --git a/src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs b/src/System.Management.Automation/engine/Subsystem/PredictionSubsystem/ICommandPredictor.cs similarity index 100% rename from src/System.Management.Automation/engine/Subsystem/CommandPrediction/ICommandPredictor.cs rename to src/System.Management.Automation/engine/Subsystem/PredictionSubsystem/ICommandPredictor.cs From 80271e25d8713b20a987c2c41d5b607de689fafb Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Sat, 24 Sep 2022 16:31:03 -0700 Subject: [PATCH 03/29] Refactor the suggestion framework --- .../host/msh/ConsoleHost.cs | 38 +- .../engine/Subsystem/ISubsystem.cs | 5 + .../engine/Subsystem/SubsystemManager.cs | 12 + .../ISuggestionProvider.cs | 338 ++++++++++++++++++ 4 files changed, 381 insertions(+), 12 deletions(-) create mode 100644 src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index cb1688d4aa0..4bb5f297145 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#pragma warning disable 1634, 1691 - using System; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -18,6 +16,7 @@ using System.Management.Automation.Remoting; using System.Management.Automation.Remoting.Server; using System.Management.Automation.Runspaces; +using System.Management.Automation.Subsystem.Suggestion; using System.Management.Automation.Tracing; using System.Reflection; using System.Runtime; @@ -2809,22 +2808,37 @@ private void EvaluateSuggestions(ConsoleHostUserInterface ui) // Output any training suggestions try { - List suggestions = HostUtilities.GetSuggestion(_parent.Runspace); - - if (suggestions.Count > 0) + List suggestions = SuggestionHub.GetSuggestions(_parent.Runspace); + if (suggestions is null || suggestions.Count == 0) { - ui.WriteLine(); + return; } - bool first = true; - foreach (string suggestion in suggestions) + ui.WriteLine(); + foreach (SuggestionEntry entry in suggestions) { - if (!first) - ui.WriteLine(); + string[] lines = entry.Text.Split( + new string[] { "\r\n", "\r", "\n" }, + StringSplitOptions.None); - ui.WriteLine(suggestion); + string output; + if (lines.Length > 1) + { + var sb = new StringBuilder(); + foreach (string line in lines) + { + sb.Append($" {line}\n"); + } + + output = sb.ToString(); + } + else + { + output = $" {lines[0]}\n"; + } - first = false; + output = $"Suggestion [{PSStyle.Instance.Foreground.BrightGreen}{entry.Name}{PSStyle.Instance.Reset}]:\n{output}"; + ui.Write(output); } } catch (TerminateException) diff --git a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs index db842f435bb..9f22e52dd16 100644 --- a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs +++ b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs @@ -22,6 +22,11 @@ public enum SubsystemKind /// Cross platform desired state configuration component. /// CrossPlatformDsc = 2, + + /// + /// Component that provides suggestion when a command fails interactively. + /// + SuggestionProvider = 3, } /// diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index d603436790c..180772ff2bc 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -10,6 +10,7 @@ using System.Management.Automation.Internal; using System.Management.Automation.Subsystem.DSC; using System.Management.Automation.Subsystem.Prediction; +using System.Management.Automation.Subsystem.Suggestion; namespace System.Management.Automation.Subsystem { @@ -35,6 +36,11 @@ static SubsystemManager() SubsystemKind.CrossPlatformDsc, allowUnregistration: true, allowMultipleRegistration: false), + + SubsystemInfo.Create( + SubsystemKind.SuggestionProvider, + allowUnregistration: true, + allowMultipleRegistration: true), }; var subSystemTypeMap = new Dictionary(subsystems.Length); @@ -49,6 +55,12 @@ static SubsystemManager() s_subsystems = new ReadOnlyCollection(subsystems); s_subSystemTypeMap = new ReadOnlyDictionary(subSystemTypeMap); s_subSystemKindMap = new ReadOnlyDictionary(subSystemKindMap); + + // Register built-in suggestion providers. + RegisterSubsystem(SubsystemKind.SuggestionProvider, new GeneralCommandErrorSuggestion()); +#if UNIX + RegisterSubsystem(SubsystemKind.SuggestionProvider, new UnixCommandNotFoundSuggestion()); +#endif } #region internal - Retrieve subsystem proxy object diff --git a/src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs b/src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs new file mode 100644 index 00000000000..d971d8ca0cd --- /dev/null +++ b/src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs @@ -0,0 +1,338 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Management.Automation.Internal; +using System.Management.Automation.Runspaces; +using System.Management.Automation.Subsystem.Prediction; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerShell.Commands; + +namespace System.Management.Automation.Subsystem.Suggestion +{ + /// + /// + /// + public interface ISuggestionProvider : ISubsystem + { + /// + /// Default implementation. No function is required for a predictor. + /// + Dictionary? ISubsystem.FunctionsToDefine => null; + + /// + /// Default implementation for `ISubsystem.Kind`. + /// + SubsystemKind ISubsystem.Kind => SubsystemKind.SuggestionProvider; + + /// + /// Gets suggestion based on the given commandline and error record. + /// + /// + string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token); + } + + internal class GeneralCommandErrorSuggestion : ISuggestionProvider + { + private readonly Guid _guid; + + internal GeneralCommandErrorSuggestion() + { + _guid = new Guid("A3C6B07E-4A89-40C9-8BE6-2A9AAD2786A4"); + } + + public Guid Id => _guid; + + public string Name => "General"; + + public string Description => "The built-in general suggestion source for command errors."; + + /// + /// + /// + public string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token) + { + var rsToUse = Runspace.DefaultRunspace; + if (rsToUse is null) + { + return null; + } + + EngineIntrinsics context = rsToUse.ExecutionContext.EngineIntrinsics; + if (lastError.FullyQualifiedErrorId == "CommandNotFoundException") + { + var target = (string)lastError.TargetObject; + CommandInvocationIntrinsics invocation = context.SessionState.InvokeCommand; + + // First, see if target is actually an executable file in current directory. + { + var localTarget = Path.Combine(".", target); + var command = invocation.GetCommand( + localTarget, + CommandTypes.Application | CommandTypes.ExternalScript); + + if (command is not null) + { + return StringUtil.Format( + SuggestionStrings.Suggestion_CommandExistsInCurrentDirectory, + target, + localTarget); + } + } + + if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) + { + var results = invocation.InvokeScript(@$" + $cmdNames = Get-Command {target} -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name + [string]::Join(', ', $cmdNames) + "); + + return StringUtil.Format( + SuggestionStrings.Suggestion_CommandNotFound, + results[0].ToString()); + } + } + + return null; + } + } + + internal class UnixCommandNotFoundSuggestion : ISuggestionProvider + { + private readonly Guid _guid; + + internal UnixCommandNotFoundSuggestion() + { + _guid = new Guid("47013747-CB9D-4EBC-9F02-F32B8AB19D48"); + } + + public Guid Id => _guid; + + public string Name => "UnixCommand"; + + public string Description => "The built-in suggestion source for Unix command utility."; + + /// + /// + /// + public string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token) + { + if (Platform.IsWindows || lastError.FullyQualifiedErrorId != "CommandNotFoundException") + { + return null; + } + + const string cmd_not_found = "/usr/lib/command-not-found"; + var target = (string)lastError.TargetObject; + var file = new FileInfo(cmd_not_found); + if (file.Exists && file.UnixFileMode.HasFlag(UnixFileMode.OtherExecute)) + { + var startInfo = new ProcessStartInfo(cmd_not_found); + startInfo.ArgumentList.Add(target); + startInfo.RedirectStandardError = true; + + var process = Process.Start(startInfo); + var output = process?.StandardError.ReadToEnd(); + return output; + } + + return null; + } + } + + /// + /// + /// + public static class SuggestionHub + { + /// + /// + /// + public static List? GetSuggestions(Runspace runspace) + { + return GetSuggestions(runspace, millisecondsTimeout: 300); + } + + /// + /// Collect the predictive suggestions from registered predictors using the specified timeout. + /// + public static List? GetSuggestions(Runspace runspace, int millisecondsTimeout) + { + Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); + + var localRunspace = runspace as LocalRunspace; + if (localRunspace is null) + { + return null; + } + + // Get the last value of $? + bool questionMarkValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; + if (questionMarkValue) + { + return null; + } + + // Get the last history item + HistoryInfo[] histories = localRunspace.History.GetEntries(id: 0, count: 1, newest: true); + if (histories.Length == 0) + { + return null; + } + + HistoryInfo lastHistory = histories[0]; + + // Get the last error + ArrayList errorList = (ArrayList)localRunspace.ExecutionContext.DollarErrorVariable; + if (errorList.Count == 0) + { + return null; + } + + var lastError = errorList[0] as ErrorRecord; + if (lastError is null && errorList[0] is RuntimeException rtEx) + { + lastError = rtEx.ErrorRecord; + } + + if (lastError?.InvocationInfo is null || lastError.InvocationInfo.HistoryId != lastHistory.Id) + { + return null; + } + + var providers = SubsystemManager.GetSubsystems(); + if (providers.Count == 0) + { + return null; + } + + int length = providers.Count; + var tasks = new List>(length); + var resultList = new List(length); + using var cancellationSource = new CancellationTokenSource(); + + ISuggestionProvider? generalSuggestionSource = null; + Func callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource); + + for (int i = 0; i < providers.Count; i++) + { + ISuggestionProvider provider = providers[i]; + if (provider is GeneralCommandErrorSuggestion) + { + length--; + generalSuggestionSource = provider; + continue; + } + + tasks.Add(Task.Factory.StartNew( + callBack, + provider, + cancellationSource.Token, + TaskCreationOptions.DenyChildAttach, + TaskScheduler.Default)); + } + + var waitTask = Task.WhenAny( + Task.WhenAll(tasks), + Task.Delay(millisecondsTimeout, cancellationSource.Token)); + + if (generalSuggestionSource is not null) + { + bool changedDefault = false; + Runspace? oldDefault = Runspace.DefaultRunspace; + + try + { + if (oldDefault != localRunspace) + { + changedDefault = true; + Runspace.DefaultRunspace = localRunspace; + } + + string? text = generalSuggestionSource.GetSuggestion(lastHistory.CommandLine, lastError, cancellationSource.Token); + if (text is not null) + { + resultList.Add(new SuggestionEntry(generalSuggestionSource.Id, generalSuggestionSource.Name, text)); + } + } + finally + { + if (changedDefault) + { + Runspace.DefaultRunspace = oldDefault; + } + + // Restore $? + localRunspace.ExecutionContext.QuestionMarkVariableValue = questionMarkValue; + } + } + + waitTask.Wait(); + cancellationSource.Cancel(); + + foreach (Task task in tasks) + { + if (task.IsCompletedSuccessfully) + { + SuggestionEntry? result = task.Result; + if (result != null) + { + resultList.Add(result); + } + } + } + + 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( + string commandLine, + ErrorRecord lastError, + CancellationTokenSource cancellationSource) + { + return state => + { + var provider = (ISuggestionProvider)state!; + var text = provider.GetSuggestion(commandLine, lastError, cancellationSource.Token); + return text is null ? null : new SuggestionEntry(provider.Id, provider.Name, text); + }; + } + } + } + + /// + /// + /// + public class SuggestionEntry + { + /// + /// Gets the Id of the predictor. + /// + public Guid Id { get; } + + /// + /// Gets the name of the predictor. + /// + public string Name { get; } + + /// + /// + /// + public string Text { get; } + + internal SuggestionEntry(Guid id, string name, string text) + { + Id = id; + Name = name; + Text = text; + } + } +} From 8c185383ff82097ecce7e29560b1850fdd76981c Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Sat, 24 Sep 2022 16:42:18 -0700 Subject: [PATCH 04/29] Minor fix --- src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 4bb5f297145..943c5af4c4a 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2819,7 +2819,7 @@ private void EvaluateSuggestions(ConsoleHostUserInterface ui) { string[] lines = entry.Text.Split( new string[] { "\r\n", "\r", "\n" }, - StringSplitOptions.None); + StringSplitOptions.RemoveEmptyEntries); string output; if (lines.Length > 1) From f33efdf373082336cde597abcc40bae9637f3b7b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Sat, 24 Sep 2022 22:30:03 -0700 Subject: [PATCH 05/29] Renaming types --- .../host/msh/ConsoleHost.cs | 6 +- .../IFeedbackProvider.cs} | 97 +++++++++++-------- .../engine/Subsystem/ISubsystem.cs | 2 +- .../engine/Subsystem/SubsystemManager.cs | 10 +- 4 files changed, 65 insertions(+), 50 deletions(-) rename src/System.Management.Automation/engine/Subsystem/{SuggestionSubsystem/ISuggestionProvider.cs => FeedbackSubsystem/IFeedbackProvider.cs} (71%) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 943c5af4c4a..f4bf4ca75e1 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -16,7 +16,7 @@ using System.Management.Automation.Remoting; using System.Management.Automation.Remoting.Server; using System.Management.Automation.Runspaces; -using System.Management.Automation.Subsystem.Suggestion; +using System.Management.Automation.Subsystem.Feedback; using System.Management.Automation.Tracing; using System.Reflection; using System.Runtime; @@ -2808,14 +2808,14 @@ private void EvaluateSuggestions(ConsoleHostUserInterface ui) // Output any training suggestions try { - List suggestions = SuggestionHub.GetSuggestions(_parent.Runspace); + List suggestions = FeedbackHub.GetFeedback(_parent.Runspace); if (suggestions is null || suggestions.Count == 0) { return; } ui.WriteLine(); - foreach (SuggestionEntry entry in suggestions) + foreach (FeedbackEntry entry in suggestions) { string[] lines = entry.Text.Split( new string[] { "\r\n", "\r", "\n" }, diff --git a/src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs similarity index 71% rename from src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs rename to src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index d971d8ca0cd..8779ed05770 100644 --- a/src/System.Management.Automation/engine/Subsystem/SuggestionSubsystem/ISuggestionProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -15,12 +15,12 @@ using System.Threading.Tasks; using Microsoft.PowerShell.Commands; -namespace System.Management.Automation.Subsystem.Suggestion +namespace System.Management.Automation.Subsystem.Feedback { /// /// /// - public interface ISuggestionProvider : ISubsystem + public interface IFeedbackProvider : ISubsystem { /// /// Default implementation. No function is required for a predictor. @@ -30,20 +30,20 @@ public interface ISuggestionProvider : ISubsystem /// /// Default implementation for `ISubsystem.Kind`. /// - SubsystemKind ISubsystem.Kind => SubsystemKind.SuggestionProvider; + SubsystemKind ISubsystem.Kind => SubsystemKind.FeedbackProvider; /// - /// Gets suggestion based on the given commandline and error record. + /// Gets feedback based on the given commandline and error record. /// /// - string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token); + string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token); } - internal class GeneralCommandErrorSuggestion : ISuggestionProvider + internal class GeneralCommandErrorFeedback : IFeedbackProvider { private readonly Guid _guid; - internal GeneralCommandErrorSuggestion() + internal GeneralCommandErrorFeedback() { _guid = new Guid("A3C6B07E-4A89-40C9-8BE6-2A9AAD2786A4"); } @@ -52,12 +52,12 @@ internal GeneralCommandErrorSuggestion() public string Name => "General"; - public string Description => "The built-in general suggestion source for command errors."; + public string Description => "The built-in general feedback source for command errors."; /// /// /// - public string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token) + public string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token) { var rsToUse = Runspace.DefaultRunspace; if (rsToUse is null) @@ -104,33 +104,43 @@ internal GeneralCommandErrorSuggestion() } } - internal class UnixCommandNotFoundSuggestion : ISuggestionProvider + internal class UnixCommandNotFound : IFeedbackProvider { private readonly Guid _guid; - internal UnixCommandNotFoundSuggestion() + internal UnixCommandNotFound() { _guid = new Guid("47013747-CB9D-4EBC-9F02-F32B8AB19D48"); } public Guid Id => _guid; - public string Name => "UnixCommand"; + public string Name => "cmd-not-found"; - public string Description => "The built-in suggestion source for Unix command utility."; + public string Description => "The built-in feedback source for the Unix command utility."; /// /// /// - public string? GetSuggestion(string commandLine, ErrorRecord lastError, CancellationToken token) + public string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token) { if (Platform.IsWindows || lastError.FullyQualifiedErrorId != "CommandNotFoundException") { return null; } - const string cmd_not_found = "/usr/lib/command-not-found"; var target = (string)lastError.TargetObject; + if (target is null) + { + return null; + } + + if (target.EndsWith(".ps1", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + const string cmd_not_found = "/usr/lib/command-not-found"; var file = new FileInfo(cmd_not_found); if (file.Exists && file.UnixFileMode.HasFlag(UnixFileMode.OtherExecute)) { @@ -139,8 +149,13 @@ internal UnixCommandNotFoundSuggestion() startInfo.RedirectStandardError = true; var process = Process.Start(startInfo); - var output = process?.StandardError.ReadToEnd(); - return output; + var output = process?.StandardError.ReadToEnd().Trim(); + + // The feedback contains recommended actions only if the output has multiple lines of text. + if (output?.IndexOfAny(new char[] { '\r', '\n' }) > 0) + { + return output; + } } return null; @@ -150,20 +165,20 @@ internal UnixCommandNotFoundSuggestion() /// /// /// - public static class SuggestionHub + public static class FeedbackHub { /// /// /// - public static List? GetSuggestions(Runspace runspace) + public static List? GetFeedback(Runspace runspace) { - return GetSuggestions(runspace, millisecondsTimeout: 300); + return GetFeedback(runspace, millisecondsTimeout: 300); } /// - /// Collect the predictive suggestions from registered predictors using the specified timeout. + /// Collect the feedback from registered feedback providers using the specified timeout. /// - public static List? GetSuggestions(Runspace runspace, int millisecondsTimeout) + public static List? GetFeedback(Runspace runspace, int millisecondsTimeout) { Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); @@ -207,27 +222,27 @@ public static class SuggestionHub return null; } - var providers = SubsystemManager.GetSubsystems(); + var providers = SubsystemManager.GetSubsystems(); if (providers.Count == 0) { return null; } int length = providers.Count; - var tasks = new List>(length); - var resultList = new List(length); + var tasks = new List>(length); + var resultList = new List(length); using var cancellationSource = new CancellationTokenSource(); - ISuggestionProvider? generalSuggestionSource = null; - Func callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource); + IFeedbackProvider? generalFeedback = null; + Func callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource); for (int i = 0; i < providers.Count; i++) { - ISuggestionProvider provider = providers[i]; - if (provider is GeneralCommandErrorSuggestion) + IFeedbackProvider provider = providers[i]; + if (provider is GeneralCommandErrorFeedback) { length--; - generalSuggestionSource = provider; + generalFeedback = provider; continue; } @@ -243,7 +258,7 @@ public static class SuggestionHub Task.WhenAll(tasks), Task.Delay(millisecondsTimeout, cancellationSource.Token)); - if (generalSuggestionSource is not null) + if (generalFeedback is not null) { bool changedDefault = false; Runspace? oldDefault = Runspace.DefaultRunspace; @@ -256,10 +271,10 @@ public static class SuggestionHub Runspace.DefaultRunspace = localRunspace; } - string? text = generalSuggestionSource.GetSuggestion(lastHistory.CommandLine, lastError, cancellationSource.Token); + string? text = generalFeedback.GetFeedback(lastHistory.CommandLine, lastError, cancellationSource.Token); if (text is not null) { - resultList.Add(new SuggestionEntry(generalSuggestionSource.Id, generalSuggestionSource.Name, text)); + resultList.Add(new FeedbackEntry(generalFeedback.Id, generalFeedback.Name, text)); } } finally @@ -277,11 +292,11 @@ public static class SuggestionHub waitTask.Wait(); cancellationSource.Cancel(); - foreach (Task task in tasks) + foreach (Task task in tasks) { if (task.IsCompletedSuccessfully) { - SuggestionEntry? result = task.Result; + FeedbackEntry? result = task.Result; if (result != null) { resultList.Add(result); @@ -293,16 +308,16 @@ public static class SuggestionHub // A local helper function to avoid creating an instance of the generated delegate helper class // when no predictor is registered. - static Func GetCallBack( + static Func GetCallBack( string commandLine, ErrorRecord lastError, CancellationTokenSource cancellationSource) { return state => { - var provider = (ISuggestionProvider)state!; - var text = provider.GetSuggestion(commandLine, lastError, cancellationSource.Token); - return text is null ? null : new SuggestionEntry(provider.Id, provider.Name, text); + var provider = (IFeedbackProvider)state!; + var text = provider.GetFeedback(commandLine, lastError, cancellationSource.Token); + return text is null ? null : new FeedbackEntry(provider.Id, provider.Name, text); }; } } @@ -311,7 +326,7 @@ public static class SuggestionHub /// /// /// - public class SuggestionEntry + public class FeedbackEntry { /// /// Gets the Id of the predictor. @@ -328,7 +343,7 @@ public class SuggestionEntry /// public string Text { get; } - internal SuggestionEntry(Guid id, string name, string text) + internal FeedbackEntry(Guid id, string name, string text) { Id = id; Name = name; diff --git a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs index 9f22e52dd16..9b4160fd560 100644 --- a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs +++ b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs @@ -26,7 +26,7 @@ public enum SubsystemKind /// /// Component that provides suggestion when a command fails interactively. /// - SuggestionProvider = 3, + FeedbackProvider = 3, } /// diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index 180772ff2bc..fc595981c75 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -10,7 +10,7 @@ using System.Management.Automation.Internal; using System.Management.Automation.Subsystem.DSC; using System.Management.Automation.Subsystem.Prediction; -using System.Management.Automation.Subsystem.Suggestion; +using System.Management.Automation.Subsystem.Feedback; namespace System.Management.Automation.Subsystem { @@ -37,8 +37,8 @@ static SubsystemManager() allowUnregistration: true, allowMultipleRegistration: false), - SubsystemInfo.Create( - SubsystemKind.SuggestionProvider, + SubsystemInfo.Create( + SubsystemKind.FeedbackProvider, allowUnregistration: true, allowMultipleRegistration: true), }; @@ -57,9 +57,9 @@ static SubsystemManager() s_subSystemKindMap = new ReadOnlyDictionary(subSystemKindMap); // Register built-in suggestion providers. - RegisterSubsystem(SubsystemKind.SuggestionProvider, new GeneralCommandErrorSuggestion()); + RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); #if UNIX - RegisterSubsystem(SubsystemKind.SuggestionProvider, new UnixCommandNotFoundSuggestion()); + RegisterSubsystem(SubsystemKind.SuggestionProvider, new UnixCommandNotFound()); #endif } From f428daf1f01d949adeb8639c62f86e2949805ddf Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Sat, 24 Sep 2022 22:34:41 -0700 Subject: [PATCH 06/29] Minor fix --- .../engine/Subsystem/SubsystemManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index fc595981c75..2de20299dce 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -59,7 +59,7 @@ static SubsystemManager() // Register built-in suggestion providers. RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); #if UNIX - RegisterSubsystem(SubsystemKind.SuggestionProvider, new UnixCommandNotFound()); + RegisterSubsystem(SubsystemKind.FeedbackProvider, new UnixCommandNotFound()); #endif } From 9b52e9e241b8d3dc368e771e65326e59e0b9ac7b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 26 Sep 2022 10:17:37 -0700 Subject: [PATCH 07/29] Make 'UnixCommandNotFound' both a predictor and a feedback provider --- .../FeedbackSubsystem/IFeedbackProvider.cs | 87 ++++++++++++++++++- .../engine/Subsystem/ISubsystem.cs | 6 +- .../engine/Subsystem/SubsystemInfo.cs | 12 +++ .../engine/Subsystem/SubsystemManager.cs | 13 +-- .../resources/SubsystemStrings.resx | 3 + 5 files changed, 111 insertions(+), 10 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 8779ed05770..bde43c1adad 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -39,7 +39,7 @@ public interface IFeedbackProvider : ISubsystem string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token); } - internal class GeneralCommandErrorFeedback : IFeedbackProvider + internal sealed class GeneralCommandErrorFeedback : IFeedbackProvider { private readonly Guid _guid; @@ -104,20 +104,28 @@ internal GeneralCommandErrorFeedback() } } - internal class UnixCommandNotFound : IFeedbackProvider + internal sealed class UnixCommandNotFound : IFeedbackProvider, ICommandPredictor { private readonly Guid _guid; + private string? _notFoundFeedback; + private List? _candidates; internal UnixCommandNotFound() { _guid = new Guid("47013747-CB9D-4EBC-9F02-F32B8AB19D48"); } + Dictionary? ISubsystem.FunctionsToDefine => null; + + SubsystemKind ISubsystem.Kind => SubsystemKind.FeedbackProvider | SubsystemKind.CommandPredictor; + public Guid Id => _guid; public string Name => "cmd-not-found"; - public string Description => "The built-in feedback source for the Unix command utility."; + public string Description => "The built-in feedback/prediction source for the Unix command utility."; + + #region IFeedbackProvider /// /// @@ -154,12 +162,85 @@ internal UnixCommandNotFound() // The feedback contains recommended actions only if the output has multiple lines of text. if (output?.IndexOfAny(new char[] { '\r', '\n' }) > 0) { + _notFoundFeedback = output; return output; } } return null; } + + #endregion + + #region ICommandPredictor + + public bool CanAcceptFeedback(PredictionClient client, PredictorFeedbackKind feedback) + { + return feedback switch + { + PredictorFeedbackKind.CommandLineAccepted => true, + _ => false, + }; + } + + public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContext context, CancellationToken cancellationToken) + { + // TODO: + // 1. text matching which is not reliable, need to re-visit. + // 2. possible race condition??? + if (_candidates is null && _notFoundFeedback is not null) + { + string[] lines = _notFoundFeedback.Split( + new char[] { '\r', '\n' }, + StringSplitOptions.RemoveEmptyEntries); + + if (lines[0].EndsWith("but can be installed with:", StringComparison.Ordinal)) + { + _candidates = new List(lines.Length); + for (int i = 1; i < lines.Length; i++) + { + _candidates.Add(lines[i]); + } + } + } + + if (_candidates is not null) + { + string input = context.InputAst.Extent.Text; + List? result = null; + + foreach (string c in _candidates) + { + if (c.StartsWith(input, StringComparison.OrdinalIgnoreCase)) + { + result ??= new List(_candidates.Count); + result.Add(new PredictiveSuggestion(c)); + } + } + + if (result is not null) + { + return new SuggestionPackage(); + } + } + + return default; + } + + public void OnCommandLineAccepted(PredictionClient client, IReadOnlyList history) + { + // Reset the candidate state. + _notFoundFeedback = null; + _candidates = null; + } + + public void OnSuggestionDisplayed(PredictionClient client, uint session, int countOrIndex) { } + + public void OnSuggestionAccepted(PredictionClient client, uint session, string acceptedSuggestion) { } + + public void OnCommandLineExecuted(PredictionClient client, string commandLine, bool success) { } + + #endregion; } /// diff --git a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs index 9b4160fd560..0a8c6f94eac 100644 --- a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs +++ b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs @@ -10,7 +10,9 @@ namespace System.Management.Automation.Subsystem { /// /// Define the kinds of subsystems. + /// Allow composite enum values to enable one subsystem implementation to serve as multiple subystems. /// + [Flags] public enum SubsystemKind { /// @@ -24,9 +26,9 @@ public enum SubsystemKind CrossPlatformDsc = 2, /// - /// Component that provides suggestion when a command fails interactively. + /// Component that provides feedback when a command fails interactively. /// - FeedbackProvider = 3, + FeedbackProvider = 4, } /// diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs index ae555e8d187..a3f14b276ce 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs @@ -78,6 +78,18 @@ public abstract class SubsystemInfo private protected SubsystemInfo(SubsystemKind kind, Type subsystemType) { + // Validate the specified enum value is not a composite value. + var value = (int)kind; + if ((value & (value - 1)) != 0) + { + // The value is a composite value because it's not power of 2. + throw new ArgumentException( + StringUtil.Format( + SubsystemStrings.CannotBeCompositeEnumValue, + kind.ToString()), + nameof(kind)); + } + _syncObj = new object(); _cachedImplInfos = Utils.EmptyReadOnlyCollection(); diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index 2de20299dce..c1dc59722ad 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -6,11 +6,11 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; -using System.Linq; using System.Management.Automation.Internal; using System.Management.Automation.Subsystem.DSC; using System.Management.Automation.Subsystem.Prediction; using System.Management.Automation.Subsystem.Feedback; +using System.Runtime.InteropServices; namespace System.Management.Automation.Subsystem { @@ -58,9 +58,12 @@ static SubsystemManager() // Register built-in suggestion providers. RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); -#if UNIX - RegisterSubsystem(SubsystemKind.FeedbackProvider, new UnixCommandNotFound()); -#endif + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + var instance = new UnixCommandNotFound(); + RegisterSubsystem(SubsystemKind.FeedbackProvider, instance); + RegisterSubsystem(SubsystemKind.CommandPredictor, instance); + } } #region internal - Retrieve subsystem proxy object @@ -194,7 +197,7 @@ public static void RegisterSubsystem(SubsystemKind kind, ISubsystem proxy) { Requires.NotNull(proxy, nameof(proxy)); - if (kind != proxy.Kind) + if (!proxy.Kind.HasFlag(kind)) { throw new ArgumentException( StringUtil.Format( diff --git a/src/System.Management.Automation/resources/SubsystemStrings.resx b/src/System.Management.Automation/resources/SubsystemStrings.resx index a2819da8682..f76e6c96dce 100644 --- a/src/System.Management.Automation/resources/SubsystemStrings.resx +++ b/src/System.Management.Automation/resources/SubsystemStrings.resx @@ -153,4 +153,7 @@ The 'Description' property of an implementation for the subsystem '{0}' cannot be null or an empty string. + + The subsystem kind should not be a composite enum value, but the specified value is '{0}'. + From 72dd6d6089a4640fe88b1e172e5a8ae47d2e0099 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 26 Sep 2022 10:31:10 -0700 Subject: [PATCH 08/29] Minor fix --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index bde43c1adad..87f2b684acb 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -199,7 +199,7 @@ public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContex _candidates = new List(lines.Length); for (int i = 1; i < lines.Length; i++) { - _candidates.Add(lines[i]); + _candidates.Add(lines[i].Trim()); } } } @@ -220,7 +220,7 @@ public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContex if (result is not null) { - return new SuggestionPackage(); + return new SuggestionPackage(result); } } From 634e2fead81f8be19c9c58154328a82b868b5e28 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 28 Sep 2022 09:20:24 -0700 Subject: [PATCH 09/29] Ignore the stdout stream --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 87f2b684acb..da19b206947 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -155,8 +155,12 @@ internal UnixCommandNotFound() var startInfo = new ProcessStartInfo(cmd_not_found); startInfo.ArgumentList.Add(target); startInfo.RedirectStandardError = true; + // The standard output stream may contain suggestions, for example, you run `python`, but you have `python3` installed, + // then the stdout will contains 'You also have python3 installed, you can run 'python3' instead'. + // But we are not using this information here. + startInfo.RedirectStandardOutput = true; - var process = Process.Start(startInfo); + using var process = Process.Start(startInfo); var output = process?.StandardError.ReadToEnd().Trim(); // The feedback contains recommended actions only if the output has multiple lines of text. From de7beebfb1288861244f3b19e4254979ec737a0c Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 28 Sep 2022 09:28:00 -0700 Subject: [PATCH 10/29] Fix fuzzy feedback --- .../FeedbackSubsystem/IFeedbackProvider.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index da19b206947..f32129f9f79 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -91,12 +91,17 @@ internal GeneralCommandErrorFeedback() { var results = invocation.InvokeScript(@$" $cmdNames = Get-Command {target} -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name - [string]::Join(', ', $cmdNames) + if ($cmdNames) {{ + [string]::Join(', ', $cmdNames) + }} "); - return StringUtil.Format( - SuggestionStrings.Suggestion_CommandNotFound, - results[0].ToString()); + if (results.Count > 0) + { + StringUtil.Format( + SuggestionStrings.Suggestion_CommandNotFound, + results[0].ToString()); + } } } From 977e15dd1b74948ca83e9e85bcbbb7c593634bba Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 28 Sep 2022 10:40:09 -0700 Subject: [PATCH 11/29] Fiz fuzzy feedback (2) --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index f32129f9f79..8676daab22e 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -98,7 +98,7 @@ internal GeneralCommandErrorFeedback() if (results.Count > 0) { - StringUtil.Format( + return StringUtil.Format( SuggestionStrings.Suggestion_CommandNotFound, results[0].ToString()); } From 9110754205bcfc91bddd340afdfefe4f319493b9 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 6 Oct 2022 17:19:54 -0700 Subject: [PATCH 12/29] Remove old suggestion code --- .../engine/hostifaces/HostUtilities.cs | 403 ------------------ 1 file changed, 403 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index 3fe4f472e1b..88f171794a0 100644 --- a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs +++ b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs @@ -42,69 +42,6 @@ public static class HostUtilities { #region Internal Access - private static readonly string s_checkForCommandInCurrentDirectoryScript = @" - [System.Diagnostics.DebuggerHidden()] - param() - - $foundSuggestion = $false - - if($lastError -and - ($lastError.Exception -is ""System.Management.Automation.CommandNotFoundException"")) - { - $escapedCommand = [System.Management.Automation.WildcardPattern]::Escape($lastError.TargetObject) - $foundSuggestion = @(Get-Command ($ExecutionContext.SessionState.Path.Combine(""."", $escapedCommand)) -ErrorAction Ignore).Count -gt 0 - } - - $foundSuggestion - "; - - private static readonly string s_createCommandExistsInCurrentDirectoryScript = @" - [System.Diagnostics.DebuggerHidden()] - param([string] $formatString) - - $formatString -f $lastError.TargetObject,"".\$($lastError.TargetObject)"" - "; - - private static readonly string s_getFuzzyMatchedCommands = @" - [System.Diagnostics.DebuggerHidden()] - param([string] $formatString) - - $formatString -f [string]::Join(', ', (Get-Command $lastError.TargetObject -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name)) - "; - - private static readonly List s_suggestions = InitializeSuggestions(); - - private static List InitializeSuggestions() - { - var suggestions = new List( - new Hashtable[] - { - NewSuggestion( - id: 3, - category: "General", - matchType: SuggestionMatchType.Dynamic, - rule: ScriptBlock.CreateDelayParsedScriptBlock(s_checkForCommandInCurrentDirectoryScript, isProductCode: true), - suggestion: ScriptBlock.CreateDelayParsedScriptBlock(s_createCommandExistsInCurrentDirectoryScript, isProductCode: true), - suggestionArgs: new object[] { CodeGeneration.EscapeSingleQuotedStringContent(SuggestionStrings.Suggestion_CommandExistsInCurrentDirectory) }, - enabled: true) - }); - - if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) - { - suggestions.Add( - NewSuggestion( - id: 4, - category: "General", - matchType: SuggestionMatchType.ErrorId, - rule: "CommandNotFoundException", - suggestion: ScriptBlock.CreateDelayParsedScriptBlock(s_getFuzzyMatchedCommands, isProductCode: true), - suggestionArgs: new object[] { CodeGeneration.EscapeSingleQuotedStringContent(SuggestionStrings.Suggestion_CommandNotFound) }, - enabled: true)); - } - - return suggestions; - } - #region GetProfileCommands /// /// Gets a PSObject whose base object is currentUserCurrentHost and with notes for the other 4 parameters. @@ -124,16 +61,6 @@ internal static PSObject GetDollarProfile(string allUsersAllHosts, string allUse return returnValue; } - /// - /// Gets an array of commands that can be run sequentially to set $profile and run the profile commands. - /// - /// The id identifying the host or shell used in profile file names. - /// - internal static PSCommand[] GetProfileCommands(string shellId) - { - return HostUtilities.GetProfileCommands(shellId, false); - } - /// /// Gets the object that serves as a value to $profile and the paths on it. /// @@ -154,42 +81,6 @@ internal static void GetProfileObjectData(string shellId, bool useTestProfile, o dollarProfile = HostUtilities.GetDollarProfile(allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost); } - /// - /// Gets an array of commands that can be run sequentially to set $profile and run the profile commands. - /// - /// The id identifying the host or shell used in profile file names. - /// Used from test not to overwrite the profile file names from development boxes. - /// - internal static PSCommand[] GetProfileCommands(string shellId, bool useTestProfile) - { - List commands = new List(); - string allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost; - PSObject dollarProfile; - HostUtilities.GetProfileObjectData(shellId, useTestProfile, out allUsersAllHosts, out allUsersCurrentHost, out currentUserAllHosts, out currentUserCurrentHost, out dollarProfile); - - PSCommand command = new PSCommand(); - command.AddCommand("set-variable"); - command.AddParameter("Name", "profile"); - command.AddParameter("Value", dollarProfile); - command.AddParameter("Option", ScopedItemOptions.None); - commands.Add(command); - - string[] profilePaths = new string[] { allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost }; - foreach (string profilePath in profilePaths) - { - if (!System.IO.File.Exists(profilePath)) - { - continue; - } - - command = new PSCommand(); - command.AddCommand(profilePath, false); - commands.Add(command); - } - - return commands.ToArray(); - } - /// /// Used to get all profile file names for the current or all hosts and for the current or all users. /// @@ -293,218 +184,6 @@ internal static string GetMaxLines(string source, int maxLines) return returnValue.ToString(); } - internal static List GetSuggestion(Runspace runspace) - { - if (!(runspace is LocalRunspace localRunspace)) { return new List(); } - - // Get the last value of $? - bool questionMarkVariableValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; - - // Get the last history item - History history = localRunspace.History; - HistoryInfo[] entries = history.GetEntries(-1, 1, true); - - if (entries.Length == 0) - return new List(); - - HistoryInfo lastHistory = entries[0]; - - // Get the last error - ArrayList errorList = (ArrayList)localRunspace.GetExecutionContext.DollarErrorVariable; - object lastError = null; - - if (errorList.Count > 0) - { - lastError = errorList[0] as Exception; - ErrorRecord lastErrorRecord = null; - - // The error was an actual ErrorRecord - if (lastError == null) - { - lastErrorRecord = errorList[0] as ErrorRecord; - } - else if (lastError is RuntimeException) - { - lastErrorRecord = ((RuntimeException)lastError).ErrorRecord; - } - - // If we got information about the error invocation, - // we can be more careful with the errors we pass along - if ((lastErrorRecord != null) && (lastErrorRecord.InvocationInfo != null)) - { - if (lastErrorRecord.InvocationInfo.HistoryId == lastHistory.Id) - lastError = lastErrorRecord; - else - lastError = null; - } - } - - Runspace oldDefault = null; - bool changedDefault = false; - if (Runspace.DefaultRunspace != runspace) - { - oldDefault = Runspace.DefaultRunspace; - changedDefault = true; - Runspace.DefaultRunspace = runspace; - } - - List suggestions = null; - - try - { - suggestions = GetSuggestion(lastHistory, lastError, errorList); - } - finally - { - if (changedDefault) - { - Runspace.DefaultRunspace = oldDefault; - } - } - - // Restore $? - localRunspace.ExecutionContext.QuestionMarkVariableValue = questionMarkVariableValue; - return suggestions; - } - - [SuppressMessage("Microsoft.Usage", "CA2208:InstantiateArgumentExceptionsCorrectly")] - internal static List GetSuggestion(HistoryInfo lastHistory, object lastError, ArrayList errorList) - { - var returnSuggestions = new List(); - - PSModuleInfo invocationModule = new PSModuleInfo(true); - invocationModule.SessionState.PSVariable.Set("lastHistory", lastHistory); - invocationModule.SessionState.PSVariable.Set("lastError", lastError); - - int initialErrorCount = 0; - - // Go through all of the suggestions - foreach (Hashtable suggestion in s_suggestions) - { - initialErrorCount = errorList.Count; - - // Make sure the rule is enabled - if (!LanguagePrimitives.IsTrue(suggestion["Enabled"])) - continue; - - SuggestionMatchType matchType = (SuggestionMatchType)LanguagePrimitives.ConvertTo( - suggestion["MatchType"], - typeof(SuggestionMatchType), - CultureInfo.InvariantCulture); - - // If this is a dynamic match, evaluate the ScriptBlock - if (matchType == SuggestionMatchType.Dynamic) - { - object result = null; - - ScriptBlock evaluator = suggestion["Rule"] as ScriptBlock; - if (evaluator == null) - { - suggestion["Enabled"] = false; - - throw new ArgumentException( - SuggestionStrings.RuleMustBeScriptBlock, "Rule"); - } - - try - { - result = invocationModule.Invoke(evaluator, null); - } - catch (Exception) - { - // Catch-all OK. This is a third-party call-out. - suggestion["Enabled"] = false; - continue; - } - - // If it returned results, evaluate its suggestion - if (LanguagePrimitives.IsTrue(result)) - { - string suggestionText = GetSuggestionText(suggestion["Suggestion"], (object[])suggestion["SuggestionArgs"], invocationModule); - - if (!string.IsNullOrEmpty(suggestionText)) - { - string returnString = string.Format( - CultureInfo.CurrentCulture, - "Suggestion [{0},{1}]: {2}", - (int)suggestion["Id"], - (string)suggestion["Category"], - suggestionText); - - returnSuggestions.Add(returnString); - } - } - } - else - { - string matchText = string.Empty; - - // Otherwise, this is a Regex match against the - // command or error - if (matchType == SuggestionMatchType.Command) - { - matchText = lastHistory.CommandLine; - } - else if (matchType == SuggestionMatchType.Error) - { - if (lastError != null) - { - Exception lastException = lastError as Exception; - if (lastException != null) - { - matchText = lastException.Message; - } - else - { - matchText = lastError.ToString(); - } - } - } - else if (matchType == SuggestionMatchType.ErrorId) - { - if (lastError != null && lastError is ErrorRecord errorRecord) - { - matchText = errorRecord.FullyQualifiedErrorId; - } - } - else - { - suggestion["Enabled"] = false; - - throw new ArgumentException( - SuggestionStrings.InvalidMatchType, - "MatchType"); - } - - // If the text matches, evaluate the suggestion - if (Regex.IsMatch(matchText, (string)suggestion["Rule"], RegexOptions.IgnoreCase)) - { - string suggestionText = GetSuggestionText(suggestion["Suggestion"], (object[])suggestion["SuggestionArgs"], invocationModule); - - if (!string.IsNullOrEmpty(suggestionText)) - { - string returnString = string.Format( - CultureInfo.CurrentCulture, - "Suggestion [{0},{1}]: {2}", - (int)suggestion["Id"], - (string)suggestion["Category"], - suggestionText); - - returnSuggestions.Add(returnString); - } - } - } - - // If the rule generated an error, disable it - if (errorList.Count != initialErrorCount) - { - suggestion["Enabled"] = false; - } - } - - return returnSuggestions; - } - /// /// Remove the GUID from the message if the message is in the pre-defined format. /// @@ -547,88 +226,6 @@ internal static string RemoveIdentifierInfoFromMessage(string message, out bool return message; } - /// - /// Create suggestion with string rule and scriptblock suggestion. - /// - /// Identifier for the suggestion. - /// Category for the suggestion. - /// Suggestion match type. - /// Rule to match. - /// Scriptblock to run that returns the suggestion. - /// Arguments to pass to suggestion scriptblock. - /// True if the suggestion is enabled. - /// Hashtable representing the suggestion. - private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, string rule, ScriptBlock suggestion, object[] suggestionArgs, bool enabled) - { - Hashtable result = new Hashtable(StringComparer.CurrentCultureIgnoreCase); - - result["Id"] = id; - result["Category"] = category; - result["MatchType"] = matchType; - result["Rule"] = rule; - result["Suggestion"] = suggestion; - result["SuggestionArgs"] = suggestionArgs; - result["Enabled"] = enabled; - - return result; - } - - /// - /// Create suggestion with scriptblock rule and suggestion. - /// - private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, ScriptBlock rule, ScriptBlock suggestion, bool enabled) - { - Hashtable result = new Hashtable(StringComparer.CurrentCultureIgnoreCase); - - result["Id"] = id; - result["Category"] = category; - result["MatchType"] = matchType; - result["Rule"] = rule; - result["Suggestion"] = suggestion; - result["Enabled"] = enabled; - - return result; - } - - /// - /// Create suggestion with scriptblock rule and scriptblock suggestion with arguments. - /// - private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, ScriptBlock rule, ScriptBlock suggestion, object[] suggestionArgs, bool enabled) - { - Hashtable result = NewSuggestion(id, category, matchType, rule, suggestion, enabled); - result.Add("SuggestionArgs", suggestionArgs); - - return result; - } - - /// - /// Get suggestion text from suggestion scriptblock with arguments. - /// - private static string GetSuggestionText(object suggestion, object[] suggestionArgs, PSModuleInfo invocationModule) - { - if (suggestion is ScriptBlock) - { - ScriptBlock suggestionScript = (ScriptBlock)suggestion; - - object result = null; - try - { - result = invocationModule.Invoke(suggestionScript, suggestionArgs); - } - catch (Exception) - { - // Catch-all OK. This is a third-party call-out. - return string.Empty; - } - - return (string)LanguagePrimitives.ConvertTo(result, typeof(string), CultureInfo.CurrentCulture); - } - else - { - return (string)LanguagePrimitives.ConvertTo(suggestion, typeof(string), CultureInfo.CurrentCulture); - } - } - /// /// Returns the prompt used in remote sessions: "[machine]: basePrompt" /// From fd47f479e4d81443082b630339ac716e72d60368 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 7 Oct 2022 15:03:13 -0700 Subject: [PATCH 13/29] cleanup - wip --- .../FeedbackSubsystem/IFeedbackProvider.cs | 66 ++++++++++++------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 8676daab22e..65fd2803a5a 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -14,6 +14,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.Commands; +using static System.Net.WebRequestMethods; namespace System.Management.Automation.Subsystem.Feedback { @@ -23,7 +24,7 @@ namespace System.Management.Automation.Subsystem.Feedback public interface IFeedbackProvider : ISubsystem { /// - /// Default implementation. No function is required for a predictor. + /// Default implementation. No function is required for a feedback provider. /// Dictionary? ISubsystem.FunctionsToDefine => null; @@ -54,9 +55,6 @@ internal GeneralCommandErrorFeedback() public string Description => "The built-in general feedback source for command errors."; - /// - /// - /// public string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token) { var rsToUse = Runspace.DefaultRunspace; @@ -71,22 +69,21 @@ internal GeneralCommandErrorFeedback() var target = (string)lastError.TargetObject; CommandInvocationIntrinsics invocation = context.SessionState.InvokeCommand; - // First, see if target is actually an executable file in current directory. - { - var localTarget = Path.Combine(".", target); - var command = invocation.GetCommand( - localTarget, - CommandTypes.Application | CommandTypes.ExternalScript); + // See if target is actually an executable file in current directory. + var localTarget = Path.Combine(".", target); + var command = invocation.GetCommand( + localTarget, + CommandTypes.Application | CommandTypes.ExternalScript); - if (command is not null) - { - return StringUtil.Format( - SuggestionStrings.Suggestion_CommandExistsInCurrentDirectory, - target, - localTarget); - } + if (command is not null) + { + return StringUtil.Format( + SuggestionStrings.Suggestion_CommandExistsInCurrentDirectory, + target, + localTarget); } + // Check fuzzy matching command names. if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) { var results = invocation.InvokeScript(@$" @@ -132,8 +129,28 @@ internal UnixCommandNotFound() #region IFeedbackProvider + private static string? GetUtilityPath() + { + string cmd_not_found = "/usr/lib/command-not-found"; + bool exist = IsFileExecutable(cmd_not_found); + + if (!exist) + { + cmd_not_found = "/usr/share/command-not-found/command-not-found"; + exist = IsFileExecutable(cmd_not_found); + } + + return exist ? cmd_not_found : null; + + static bool IsFileExecutable(string path) + { + var file = new FileInfo(path); + return file.Exists && file.UnixFileMode.HasFlag(UnixFileMode.OtherExecute); + } + } + /// - /// + /// Gets feedback based on the given commandline and error record. /// public string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token) { @@ -153,9 +170,8 @@ internal UnixCommandNotFound() return null; } - const string cmd_not_found = "/usr/lib/command-not-found"; - var file = new FileInfo(cmd_not_found); - if (file.Exists && file.UnixFileMode.HasFlag(UnixFileMode.OtherExecute)) + string? cmd_not_found = GetUtilityPath(); + if (cmd_not_found is not null) { var startInfo = new ProcessStartInfo(cmd_not_found); startInfo.ArgumentList.Add(target); @@ -169,7 +185,7 @@ internal UnixCommandNotFound() var output = process?.StandardError.ReadToEnd().Trim(); // The feedback contains recommended actions only if the output has multiple lines of text. - if (output?.IndexOfAny(new char[] { '\r', '\n' }) > 0) + if (output?.AsSpan().IndexOfAny('\r', '\n') > 0) { _notFoundFeedback = output; return output; @@ -397,7 +413,7 @@ public static class FeedbackHub return resultList; // A local helper function to avoid creating an instance of the generated delegate helper class - // when no predictor is registered. + // when no feedback provider is registered. static Func GetCallBack( string commandLine, ErrorRecord lastError, @@ -419,12 +435,12 @@ public static class FeedbackHub public class FeedbackEntry { /// - /// Gets the Id of the predictor. + /// Gets the Id of the feedback provider. /// public Guid Id { get; } /// - /// Gets the name of the predictor. + /// Gets the name of the feedback provider. /// public string Name { get; } From 7a9f00e8d1cff2e37ffa255027c955359ca33ed2 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 10 Oct 2022 11:18:28 -0700 Subject: [PATCH 14/29] Cleanup - done --- .../host/msh/ConsoleHost.cs | 53 ++-- .../FeedbackSubsystem/FeedbackHub.cs | 228 +++++++++++++++ .../FeedbackSubsystem/IFeedbackProvider.cs | 260 +++--------------- 3 files changed, 295 insertions(+), 246 deletions(-) create mode 100644 src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/FeedbackHub.cs diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index f4bf4ca75e1..2464a10dde2 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2500,7 +2500,7 @@ internal void Run(bool inputLoopIsNested) // Evaluate any suggestions if (previousResponseWasEmpty == false) { - EvaluateSuggestions(ui); + EvaluateFeedbacks(ui); } // Then output the prompt @@ -2803,48 +2803,43 @@ private static bool IsIncompleteParseException(Exception e) return remoteException.ErrorRecord.CategoryInfo.Reason == nameof(IncompleteParseException); } - private void EvaluateSuggestions(ConsoleHostUserInterface ui) + private void EvaluateFeedbacks(ConsoleHostUserInterface ui) { // Output any training suggestions try { - List suggestions = FeedbackHub.GetFeedback(_parent.Runspace); - if (suggestions is null || suggestions.Count == 0) + List feedbacks = FeedbackHub.GetFeedback(_parent.Runspace); + if (feedbacks is null || feedbacks.Count == 0) { return; } + // Feedback section starts with a new line. ui.WriteLine(); - foreach (FeedbackEntry entry in suggestions) - { - string[] lines = entry.Text.Split( - new string[] { "\r\n", "\r", "\n" }, - StringSplitOptions.RemoveEmptyEntries); - - string output; - if (lines.Length > 1) - { - var sb = new StringBuilder(); - foreach (string line in lines) - { - sb.Append($" {line}\n"); - } - output = sb.ToString(); - } - else + const string Indentation = " "; + foreach (FeedbackEntry entry in feedbacks) + { + var output = new StringBuilder() + .Append("Suggestion [") + .Append(PSStyle.Instance.Foreground.BrightGreen) + .Append(entry.Name) + .Append(PSStyle.Instance.Reset) + .AppendLine("]:"); + + string[] lines = entry.Text.Split('\n'); + foreach (string line in lines) { - output = $" {lines[0]}\n"; + output.Append(Indentation) + .Append(line.AsSpan().TrimEnd()) + .AppendLine(); } - output = $"Suggestion [{PSStyle.Instance.Foreground.BrightGreen}{entry.Name}{PSStyle.Instance.Reset}]:\n{output}"; - ui.Write(output); + ui.Write(output.ToString()); } - } - catch (TerminateException) - { - // A variable breakpoint may be hit by HostUtilities.GetSuggestion. The debugger throws TerminateExceptions to stop the execution - // of the current statement; we do not want to treat these exceptions as errors. + + // Feedback section ends with a new line. + ui.WriteLine(); } catch (Exception e) { diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/FeedbackHub.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/FeedbackHub.cs new file mode 100644 index 00000000000..5755945dcd0 --- /dev/null +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/FeedbackHub.cs @@ -0,0 +1,228 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Management.Automation.Internal; +using System.Management.Automation.Runspaces; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerShell.Commands; + +namespace System.Management.Automation.Subsystem.Feedback +{ + /// + /// The class represents a result from a feedback provider. + /// + public class FeedbackEntry + { + /// + /// Gets the Id of the feedback provider. + /// + public Guid Id { get; } + + /// + /// Gets the name of the feedback provider. + /// + public string Name { get; } + + /// + /// Gets the text of the feedback. + /// + public string Text { get; } + + internal FeedbackEntry(Guid id, string name, string text) + { + Id = id; + Name = name; + Text = text; + } + } + + /// + /// Provides a set of feedbacks for given input. + /// + public static class FeedbackHub + { + /// + /// Collect the feedback from registered feedback providers using the default timeout. + /// + public static List? GetFeedback(Runspace runspace) + { + return GetFeedback(runspace, millisecondsTimeout: 300); + } + + /// + /// Collect the feedback from registered feedback providers using the specified timeout. + /// + public static List? GetFeedback(Runspace runspace, int millisecondsTimeout) + { + Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); + + var localRunspace = runspace as LocalRunspace; + if (localRunspace is null) + { + return null; + } + + // Get the last value of $? + bool questionMarkValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; + if (questionMarkValue) + { + return null; + } + + // Get the last history item + HistoryInfo[] histories = localRunspace.History.GetEntries(id: 0, count: 1, newest: true); + if (histories.Length == 0) + { + return null; + } + + HistoryInfo lastHistory = histories[0]; + + // Get the last error + ArrayList errorList = (ArrayList)localRunspace.ExecutionContext.DollarErrorVariable; + if (errorList.Count == 0) + { + return null; + } + + var lastError = errorList[0] as ErrorRecord; + if (lastError is null && errorList[0] is RuntimeException rtEx) + { + lastError = rtEx.ErrorRecord; + } + + if (lastError?.InvocationInfo is null || lastError.InvocationInfo.HistoryId != lastHistory.Id) + { + return null; + } + + var providers = SubsystemManager.GetSubsystems(); + int count = providers.Count; + if (count == 0) + { + return null; + } + + IFeedbackProvider? generalFeedback = null; + List>? tasks = null; + CancellationTokenSource? cancellationSource = null; + Func? callBack = null; + + for (int i = 0; i < providers.Count; i++) + { + IFeedbackProvider provider = providers[i]; + if (provider is GeneralCommandErrorFeedback) + { + // This built-in feedback provider needs to run on the target Runspace. + generalFeedback = provider; + continue; + } + + if (tasks is null) + { + tasks = new List>(capacity: count); + cancellationSource = new CancellationTokenSource(); + callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource); + } + + // Other feedback providers will run on background threads in parallel. + tasks.Add(Task.Factory.StartNew( + callBack!, + provider, + cancellationSource!.Token, + TaskCreationOptions.DenyChildAttach, + TaskScheduler.Default)); + } + + Task? waitTask = null; + if (tasks is not null) + { + waitTask = Task.WhenAny( + Task.WhenAll(tasks), + Task.Delay(millisecondsTimeout, cancellationSource!.Token)); + } + + List? resultList = null; + if (generalFeedback is not null) + { + bool changedDefault = false; + Runspace? oldDefault = Runspace.DefaultRunspace; + + try + { + if (oldDefault != localRunspace) + { + changedDefault = true; + Runspace.DefaultRunspace = localRunspace; + } + + string? text = generalFeedback.GetFeedback(lastHistory.CommandLine, lastError, CancellationToken.None); + if (text is not null) + { + resultList ??= new List(count); + resultList.Add(new FeedbackEntry(generalFeedback.Id, generalFeedback.Name, text)); + } + } + finally + { + if (changedDefault) + { + Runspace.DefaultRunspace = oldDefault; + } + + // Restore $? for the target Runspace. + localRunspace.ExecutionContext.QuestionMarkVariableValue = questionMarkValue; + } + } + + if (waitTask is not null) + { + try + { + waitTask.Wait(); + cancellationSource!.Cancel(); + + foreach (Task task in tasks!) + { + if (task.IsCompletedSuccessfully) + { + FeedbackEntry? result = task.Result; + if (result is not null) + { + resultList ??= new List(count); + resultList.Add(result); + } + } + } + } + finally + { + cancellationSource!.Dispose(); + } + } + + return resultList; + } + + // A local helper function to avoid creating an instance of the generated delegate helper class + // when no feedback provider is registered. + private static Func GetCallBack( + string commandLine, + ErrorRecord lastError, + CancellationTokenSource cancellationSource) + { + return state => + { + var provider = (IFeedbackProvider)state!; + var text = provider.GetFeedback(commandLine, lastError, cancellationSource.Token); + return text is null ? null : new FeedbackEntry(provider.Id, provider.Name, text); + }; + } + } +} diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 65fd2803a5a..39f9491cf2f 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -4,7 +4,6 @@ #nullable enable using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -12,14 +11,11 @@ using System.Management.Automation.Runspaces; using System.Management.Automation.Subsystem.Prediction; using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerShell.Commands; -using static System.Net.WebRequestMethods; namespace System.Management.Automation.Subsystem.Feedback { /// - /// + /// Interface for implementing a feedback provider on command failures. /// public interface IFeedbackProvider : ISubsystem { @@ -43,10 +39,13 @@ public interface IFeedbackProvider : ISubsystem internal sealed class GeneralCommandErrorFeedback : IFeedbackProvider { private readonly Guid _guid; + private readonly object[] _args; + private ScriptBlock? _fuzzySb; internal GeneralCommandErrorFeedback() { _guid = new Guid("A3C6B07E-4A89-40C9-8BE6-2A9AAD2786A4"); + _args = new object[1]; } public Guid Id => _guid; @@ -63,9 +62,10 @@ internal GeneralCommandErrorFeedback() return null; } - EngineIntrinsics context = rsToUse.ExecutionContext.EngineIntrinsics; if (lastError.FullyQualifiedErrorId == "CommandNotFoundException") { + EngineIntrinsics context = rsToUse.ExecutionContext.EngineIntrinsics; + var target = (string)lastError.TargetObject; CommandInvocationIntrinsics invocation = context.SessionState.InvokeCommand; @@ -86,18 +86,22 @@ internal GeneralCommandErrorFeedback() // Check fuzzy matching command names. if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) { - var results = invocation.InvokeScript(@$" - $cmdNames = Get-Command {target} -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name + _fuzzySb ??= ScriptBlock.CreateDelayParsedScriptBlock(@$" + param([string] $target) + $cmdNames = Get-Command $target -UseFuzzyMatch | Select-Object -First 10 -ExpandProperty Name if ($cmdNames) {{ [string]::Join(', ', $cmdNames) }} - "); + ", isProductCode: true); + + _args[0] = target; + var result = _fuzzySb.InvokeReturnAsIs(_args); - if (results.Count > 0) + if (result is not null) { return StringUtil.Format( SuggestionStrings.Suggestion_CommandNotFound, - results[0].ToString()); + result.ToString()); } } } @@ -176,19 +180,18 @@ static bool IsFileExecutable(string path) var startInfo = new ProcessStartInfo(cmd_not_found); startInfo.ArgumentList.Add(target); startInfo.RedirectStandardError = true; - // The standard output stream may contain suggestions, for example, you run `python`, but you have `python3` installed, - // then the stdout will contains 'You also have python3 installed, you can run 'python3' instead'. - // But we are not using this information here. startInfo.RedirectStandardOutput = true; using var process = Process.Start(startInfo); - var output = process?.StandardError.ReadToEnd().Trim(); + var stderr = process?.StandardError.ReadToEnd().Trim(); // The feedback contains recommended actions only if the output has multiple lines of text. - if (output?.AsSpan().IndexOfAny('\r', '\n') > 0) + if (stderr?.IndexOf('\n') > 0) { - _notFoundFeedback = output; - return output; + _notFoundFeedback = stderr; + + var stdout = process?.StandardOutput.ReadToEnd().Trim(); + return stdout is null ? stderr : $"{stderr}\n\n{stdout}"; } } @@ -210,22 +213,34 @@ public bool CanAcceptFeedback(PredictionClient client, PredictorFeedbackKind fee public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContext context, CancellationToken cancellationToken) { - // TODO: - // 1. text matching which is not reliable, need to re-visit. - // 2. possible race condition??? if (_candidates is null && _notFoundFeedback is not null) { - string[] lines = _notFoundFeedback.Split( - new char[] { '\r', '\n' }, - StringSplitOptions.RemoveEmptyEntries); + var text = _notFoundFeedback.AsSpan(); + // Set to null to avoid potential race condition. + _notFoundFeedback = null; - if (lines[0].EndsWith("but can be installed with:", StringComparison.Ordinal)) + // This loop searches for candidate results with almost no allocation. + while (true) { - _candidates = new List(lines.Length); - for (int i = 1; i < lines.Length; i++) + // The line is a candidate if it starts with "sudo ", such as "sudo apt install python3". + // 'sudo' is a command name that remains the same, so this check should work for all locales. + bool isCandidate = text.StartsWith("sudo "); + int index = text.IndexOf('\n'); + if (isCandidate) { - _candidates.Add(lines[i].Trim()); + var line = index != -1 ? text.Slice(0, index) : text; + _candidates ??= new List(); + _candidates.Add(new string(line.TrimEnd())); } + + // Break out the loop if we are done with the last line. + if (index == -1 || index == text.Length - 1) + { + break; + } + + // Point to the rest of feedback text. + text = text.Slice(index + 1); } } @@ -267,193 +282,4 @@ public void OnCommandLineExecuted(PredictionClient client, string commandLine, b #endregion; } - - /// - /// - /// - public static class FeedbackHub - { - /// - /// - /// - public static List? GetFeedback(Runspace runspace) - { - return GetFeedback(runspace, millisecondsTimeout: 300); - } - - /// - /// Collect the feedback from registered feedback providers using the specified timeout. - /// - public static List? GetFeedback(Runspace runspace, int millisecondsTimeout) - { - Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout)); - - var localRunspace = runspace as LocalRunspace; - if (localRunspace is null) - { - return null; - } - - // Get the last value of $? - bool questionMarkValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; - if (questionMarkValue) - { - return null; - } - - // Get the last history item - HistoryInfo[] histories = localRunspace.History.GetEntries(id: 0, count: 1, newest: true); - if (histories.Length == 0) - { - return null; - } - - HistoryInfo lastHistory = histories[0]; - - // Get the last error - ArrayList errorList = (ArrayList)localRunspace.ExecutionContext.DollarErrorVariable; - if (errorList.Count == 0) - { - return null; - } - - var lastError = errorList[0] as ErrorRecord; - if (lastError is null && errorList[0] is RuntimeException rtEx) - { - lastError = rtEx.ErrorRecord; - } - - if (lastError?.InvocationInfo is null || lastError.InvocationInfo.HistoryId != lastHistory.Id) - { - return null; - } - - var providers = SubsystemManager.GetSubsystems(); - if (providers.Count == 0) - { - return null; - } - - int length = providers.Count; - var tasks = new List>(length); - var resultList = new List(length); - using var cancellationSource = new CancellationTokenSource(); - - IFeedbackProvider? generalFeedback = null; - Func callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource); - - for (int i = 0; i < providers.Count; i++) - { - IFeedbackProvider provider = providers[i]; - if (provider is GeneralCommandErrorFeedback) - { - length--; - generalFeedback = provider; - continue; - } - - tasks.Add(Task.Factory.StartNew( - callBack, - provider, - cancellationSource.Token, - TaskCreationOptions.DenyChildAttach, - TaskScheduler.Default)); - } - - var waitTask = Task.WhenAny( - Task.WhenAll(tasks), - Task.Delay(millisecondsTimeout, cancellationSource.Token)); - - if (generalFeedback is not null) - { - bool changedDefault = false; - Runspace? oldDefault = Runspace.DefaultRunspace; - - try - { - if (oldDefault != localRunspace) - { - changedDefault = true; - Runspace.DefaultRunspace = localRunspace; - } - - string? text = generalFeedback.GetFeedback(lastHistory.CommandLine, lastError, cancellationSource.Token); - if (text is not null) - { - resultList.Add(new FeedbackEntry(generalFeedback.Id, generalFeedback.Name, text)); - } - } - finally - { - if (changedDefault) - { - Runspace.DefaultRunspace = oldDefault; - } - - // Restore $? - localRunspace.ExecutionContext.QuestionMarkVariableValue = questionMarkValue; - } - } - - waitTask.Wait(); - cancellationSource.Cancel(); - - foreach (Task task in tasks) - { - if (task.IsCompletedSuccessfully) - { - FeedbackEntry? result = task.Result; - if (result != null) - { - resultList.Add(result); - } - } - } - - return resultList; - - // A local helper function to avoid creating an instance of the generated delegate helper class - // when no feedback provider is registered. - static Func GetCallBack( - string commandLine, - ErrorRecord lastError, - CancellationTokenSource cancellationSource) - { - return state => - { - var provider = (IFeedbackProvider)state!; - var text = provider.GetFeedback(commandLine, lastError, cancellationSource.Token); - return text is null ? null : new FeedbackEntry(provider.Id, provider.Name, text); - }; - } - } - } - - /// - /// - /// - public class FeedbackEntry - { - /// - /// Gets the Id of the feedback provider. - /// - public Guid Id { get; } - - /// - /// Gets the name of the feedback provider. - /// - public string Name { get; } - - /// - /// - /// - public string Text { get; } - - internal FeedbackEntry(Guid id, string name, string text) - { - Id = id; - Name = name; - Text = text; - } - } } From 2a5b4bae326023395e0719b13121b1eaca490619 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 10 Oct 2022 12:08:49 -0700 Subject: [PATCH 15/29] Fix an error --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 39f9491cf2f..d7cc7a513b7 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -191,7 +191,7 @@ static bool IsFileExecutable(string path) _notFoundFeedback = stderr; var stdout = process?.StandardOutput.ReadToEnd().Trim(); - return stdout is null ? stderr : $"{stderr}\n\n{stdout}"; + return stdout == string.Empty ? stderr : $"{stderr}\n\n{stdout}"; } } From 676c6c353751666c6776153b0254e7359450f2a0 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 10 Oct 2022 13:52:54 -0700 Subject: [PATCH 16/29] Update the suggest layout --- .../host/msh/ConsoleHost.cs | 15 ++++++++++++--- .../FeedbackSubsystem/IFeedbackProvider.cs | 4 ++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 2464a10dde2..0a7cefe0ccf 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2818,16 +2818,23 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) ui.WriteLine(); const string Indentation = " "; + int count = 0; + var output = new StringBuilder(); + foreach (FeedbackEntry entry in feedbacks) { - var output = new StringBuilder() - .Append("Suggestion [") + if (count > 0) + { + output.AppendLine(); + } + + output.Append("Suggestion [") .Append(PSStyle.Instance.Foreground.BrightGreen) .Append(entry.Name) .Append(PSStyle.Instance.Reset) .AppendLine("]:"); - string[] lines = entry.Text.Split('\n'); + string[] lines = entry.Text.Split('\n', StringSplitOptions.RemoveEmptyEntries); foreach (string line in lines) { output.Append(Indentation) @@ -2836,6 +2843,8 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) } ui.Write(output.ToString()); + count++; + output.Clear(); } // Feedback section ends with a new line. diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index d7cc7a513b7..bfb605c5697 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -97,7 +97,7 @@ internal GeneralCommandErrorFeedback() _args[0] = target; var result = _fuzzySb.InvokeReturnAsIs(_args); - if (result is not null) + if (result is not null && result != AutomationNull.Value) { return StringUtil.Format( SuggestionStrings.Suggestion_CommandNotFound, @@ -191,7 +191,7 @@ static bool IsFileExecutable(string path) _notFoundFeedback = stderr; var stdout = process?.StandardOutput.ReadToEnd().Trim(); - return stdout == string.Empty ? stderr : $"{stderr}\n\n{stdout}"; + return stdout == string.Empty ? stderr : $"{stderr}\n{stdout}"; } } From 47cf37be0244c162ea8a02961c70f8e68d0688ec Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 10 Oct 2022 14:50:49 -0700 Subject: [PATCH 17/29] Minor fix --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index bfb605c5697..85e03df40e2 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -191,7 +191,7 @@ static bool IsFileExecutable(string path) _notFoundFeedback = stderr; var stdout = process?.StandardOutput.ReadToEnd().Trim(); - return stdout == string.Empty ? stderr : $"{stderr}\n{stdout}"; + return string.IsNullOrEmpty(stdout) ? stderr : $"{stderr}\n{stdout}"; } } From 3efddaaa0e621618b56186dbd398c6707da42230 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 10:16:47 -0700 Subject: [PATCH 18/29] Fix tests --- .../engine/Subsystem/ISubsystem.cs | 2 +- .../engine/Subsystem/SubsystemInfo.cs | 12 +- .../engine/Subsystem/SubsystemManager.cs | 5 + .../engine/Utils.cs | 14 ++ .../resources/SubsystemStrings.resx | 4 +- test/xUnit/csharp/test_Subsystem.cs | 183 +++++++++++++----- 6 files changed, 159 insertions(+), 61 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs index 0a8c6f94eac..e8937486d41 100644 --- a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs +++ b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs @@ -13,7 +13,7 @@ namespace System.Management.Automation.Subsystem /// Allow composite enum values to enable one subsystem implementation to serve as multiple subystems. /// [Flags] - public enum SubsystemKind + public enum SubsystemKind : uint { /// /// Component that provides predictive suggestions to commandline input. diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs index a3f14b276ce..e8d8fe39eae 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs @@ -78,17 +78,7 @@ public abstract class SubsystemInfo private protected SubsystemInfo(SubsystemKind kind, Type subsystemType) { - // Validate the specified enum value is not a composite value. - var value = (int)kind; - if ((value & (value - 1)) != 0) - { - // The value is a composite value because it's not power of 2. - throw new ArgumentException( - StringUtil.Format( - SubsystemStrings.CannotBeCompositeEnumValue, - kind.ToString()), - nameof(kind)); - } + Requires.OneSpecificSubsystemKind(kind); _syncObj = new object(); _cachedImplInfos = Utils.EmptyReadOnlyCollection(); diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index c1dc59722ad..e4658bbdfad 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -158,6 +158,8 @@ public static SubsystemInfo GetSubsystemInfo(Type subsystemType) /// The object that represents the concrete subsystem. public static SubsystemInfo GetSubsystemInfo(SubsystemKind kind) { + Requires.OneSpecificSubsystemKind(kind); + if (s_subSystemKindMap.TryGetValue(kind, out SubsystemInfo? subsystemInfo)) { return subsystemInfo; @@ -196,6 +198,7 @@ public static void RegisterSubsystem(TImple public static void RegisterSubsystem(SubsystemKind kind, ISubsystem proxy) { Requires.NotNull(proxy, nameof(proxy)); + Requires.OneSpecificSubsystemKind(kind); if (!proxy.Kind.HasFlag(kind)) { @@ -282,6 +285,8 @@ public static void UnregisterSubsystem(Guid id) /// The Id of the implementation to be unregistered. public static void UnregisterSubsystem(SubsystemKind kind, Guid id) { + Requires.OneSpecificSubsystemKind(kind); + UnregisterSubsystem(GetSubsystemInfo(kind), id); } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index a3b6b83dc9a..57463a6f330 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1788,5 +1788,19 @@ internal static void Condition([DoesNotReturnIf(false)] bool precondition, strin throw new ArgumentException(paramName); } } + + internal static void OneSpecificSubsystemKind(Subsystem.SubsystemKind kind) + { + uint value = (uint)kind; + if (value == 0 || (value & (value - 1)) != 0) + { + // The value is either invalid or a composite value because it's not power of 2. + throw new ArgumentException( + StringUtil.Format( + SubsystemStrings.RequireOneSpecificSubsystemKind, + kind.ToString()), + nameof(kind)); + } + } } } diff --git a/src/System.Management.Automation/resources/SubsystemStrings.resx b/src/System.Management.Automation/resources/SubsystemStrings.resx index f76e6c96dce..1ca0453ae31 100644 --- a/src/System.Management.Automation/resources/SubsystemStrings.resx +++ b/src/System.Management.Automation/resources/SubsystemStrings.resx @@ -153,7 +153,7 @@ The 'Description' property of an implementation for the subsystem '{0}' cannot be null or an empty string. - - The subsystem kind should not be a composite enum value, but the specified value is '{0}'. + + The subsystem kind here should represent one specific subsystem, but the given value is either invalid or a composite enum value: '{0}'. diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index d5bc3b9c728..473ff51f87b 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -6,7 +6,9 @@ using System.Management.Automation.Subsystem; using System.Management.Automation.Subsystem.DSC; using System.Management.Automation.Subsystem.Prediction; +using System.Management.Automation.Subsystem.Feedback; using System.Threading; +using System.Runtime.InteropServices; using Xunit; namespace PSTests.Sequential @@ -41,36 +43,81 @@ private static void VerifyCrossPlatformDscMetadata(SubsystemInfo ssInfo) Assert.Empty(ssInfo.RequiredFunctions); } + private static void VerifyFeedbackProviderMetadata(SubsystemInfo ssInfo) + { + Assert.Equal(SubsystemKind.FeedbackProvider, ssInfo.Kind); + Assert.Equal(typeof(IFeedbackProvider), ssInfo.SubsystemType); + Assert.True(ssInfo.AllowUnregistration); + Assert.True(ssInfo.AllowMultipleRegistration); + Assert.Empty(ssInfo.RequiredCmdlets); + Assert.Empty(ssInfo.RequiredFunctions); + } + [Fact] public static void GetSubsystemInfo() { + #region Predictor SubsystemInfo predictorInfo = SubsystemManager.GetSubsystemInfo(typeof(ICommandPredictor)); - - VerifyCommandPredictorMetadata(predictorInfo); - Assert.False(predictorInfo.IsRegistered); - Assert.Empty(predictorInfo.Implementations); - SubsystemInfo predictorInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); Assert.Same(predictorInfo2, predictorInfo); + VerifyCommandPredictorMetadata(predictorInfo); + #endregion - SubsystemInfo crossPlatformDscInfo = SubsystemManager.GetSubsystemInfo(typeof(ICrossPlatformDsc)); + #region Feedback + SubsystemInfo feedbackProviderInfo = SubsystemManager.GetSubsystemInfo(typeof(IFeedbackProvider)); + SubsystemInfo feedback2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.FeedbackProvider); + Assert.Same(feedback2, feedbackProviderInfo); + VerifyFeedbackProviderMetadata(feedbackProviderInfo); + #endregion + #region DSC + SubsystemInfo crossPlatformDscInfo = SubsystemManager.GetSubsystemInfo(typeof(ICrossPlatformDsc)); + SubsystemInfo crossPlatformDscInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CrossPlatformDsc); + Assert.Same(crossPlatformDscInfo2, crossPlatformDscInfo); VerifyCrossPlatformDscMetadata(crossPlatformDscInfo); + Assert.False(crossPlatformDscInfo.IsRegistered); Assert.Empty(crossPlatformDscInfo.Implementations); - - SubsystemInfo crossPlatformDscInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CrossPlatformDsc); - Assert.Same(crossPlatformDscInfo2, crossPlatformDscInfo); + #endregion ReadOnlyCollection ssInfos = SubsystemManager.GetAllSubsystemInfo(); - Assert.Equal(2, ssInfos.Count); + Assert.Equal(3, ssInfos.Count); Assert.Same(ssInfos[0], predictorInfo); Assert.Same(ssInfos[1], crossPlatformDscInfo); + Assert.Same(ssInfos[2], feedbackProviderInfo); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + Assert.True(predictorInfo.IsRegistered); + Assert.Single(predictorInfo.Implementations); + + Assert.True(feedbackProviderInfo.IsRegistered); + Assert.Equal(2, feedbackProviderInfo.Implementations.Count); + + ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); + Assert.NotNull(predictorImpl); + ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); + Assert.Single(predictorImpls); + + ReadOnlyCollection feedbackImpls = SubsystemManager.GetSubsystems(); + Assert.Equal(2, feedbackImpls.Count); + } + else + { + Assert.False(predictorInfo.IsRegistered); + Assert.Empty(predictorInfo.Implementations); - ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); - Assert.Null(predictorImpl); - ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); - Assert.Empty(predictorImpls); + Assert.True(feedbackProviderInfo.IsRegistered); + Assert.Single(feedbackProviderInfo.Implementations); + + ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); + Assert.Null(predictorImpl); + ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); + Assert.Empty(predictorImpls); + + ReadOnlyCollection feedbackImpls = SubsystemManager.GetSubsystems(); + Assert.Single(feedbackImpls); + } ICrossPlatformDsc crossPlatformDscImpl = SubsystemManager.GetSubsystem(); Assert.Null(crossPlatformDscImpl); @@ -81,6 +128,19 @@ public static void GetSubsystemInfo() [Fact] public static void RegisterSubsystem() { + while (!System.Diagnostics.Debugger.IsAttached) + { + System.Threading.Thread.Sleep(200); + } + System.Diagnostics.Debugger.Break(); + + // Unregister existing predictors, to make it easier to test. + var existingPredictors = SubsystemManager.GetSubsystems(); + foreach (ICommandPredictor p in existingPredictors) + { + SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, p.Id); + } + try { Assert.Throws( @@ -91,14 +151,19 @@ public static void RegisterSubsystem() () => SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, null)); Assert.Throws( paramName: "proxy", + () => SubsystemManager.RegisterSubsystem(SubsystemKind.CrossPlatformDsc, predictor1)); + Assert.Throws( + paramName: "kind", () => SubsystemManager.RegisterSubsystem((SubsystemKind)0, predictor1)); + Assert.Throws( + paramName: "kind", + () => SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor | SubsystemKind.FeedbackProvider, predictor1)); // Register 'predictor1' SubsystemManager.RegisterSubsystem(predictor1); // Now validate the SubsystemInfo of the 'ICommandPredictor' subsystem SubsystemInfo ssInfo = SubsystemManager.GetSubsystemInfo(typeof(ICommandPredictor)); - SubsystemInfo crossPlatformDscInfo = SubsystemManager.GetSubsystemInfo(typeof(ICrossPlatformDsc)); VerifyCommandPredictorMetadata(ssInfo); Assert.True(ssInfo.IsRegistered); Assert.Single(ssInfo.Implementations); @@ -160,55 +225,79 @@ public static void RegisterSubsystem() { SubsystemManager.UnregisterSubsystem(predictor1.Id); SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); + + // Re-register the pre-existing predictors. + foreach (ICommandPredictor p in existingPredictors) + { + SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, p); + } } } [Fact] public static void UnregisterSubsystem() { - // Exception expected when no implementation is registered - Assert.Throws(() => SubsystemManager.UnregisterSubsystem(predictor1.Id)); + // Unregister existing predictors, to make it easier to test. + var existingPredictors = SubsystemManager.GetSubsystems(); + foreach (ICommandPredictor p in existingPredictors) + { + SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, p.Id); + } - SubsystemManager.RegisterSubsystem(predictor1); - SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, predictor2); + try + { + // Exception expected when no implementation is registered + Assert.Throws(() => SubsystemManager.UnregisterSubsystem(predictor1.Id)); - // Exception is expected when specified id cannot be found - Assert.Throws(() => SubsystemManager.UnregisterSubsystem(Guid.NewGuid())); + SubsystemManager.RegisterSubsystem(predictor1); + SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, predictor2); - // Unregister 'predictor1' - SubsystemManager.UnregisterSubsystem(predictor1.Id); + // Exception is expected when specified id cannot be found + Assert.Throws(() => SubsystemManager.UnregisterSubsystem(Guid.NewGuid())); - SubsystemInfo ssInfo = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); - VerifyCommandPredictorMetadata(ssInfo); - Assert.True(ssInfo.IsRegistered); - Assert.Single(ssInfo.Implementations); + // Unregister 'predictor1' + SubsystemManager.UnregisterSubsystem(predictor1.Id); - var implInfo = ssInfo.Implementations[0]; - Assert.Equal(predictor2.Id, implInfo.Id); - Assert.Equal(predictor2.Name, implInfo.Name); - Assert.Equal(predictor2.Description, implInfo.Description); - Assert.Equal(SubsystemKind.CommandPredictor, implInfo.Kind); - Assert.Same(typeof(MyPredictor), implInfo.ImplementationType); + SubsystemInfo ssInfo = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); + VerifyCommandPredictorMetadata(ssInfo); + Assert.True(ssInfo.IsRegistered); + Assert.Single(ssInfo.Implementations); - ICommandPredictor impl = SubsystemManager.GetSubsystem(); - Assert.Same(impl, predictor2); + var implInfo = ssInfo.Implementations[0]; + Assert.Equal(predictor2.Id, implInfo.Id); + Assert.Equal(predictor2.Name, implInfo.Name); + Assert.Equal(predictor2.Description, implInfo.Description); + Assert.Equal(SubsystemKind.CommandPredictor, implInfo.Kind); + Assert.Same(typeof(MyPredictor), implInfo.ImplementationType); + + ICommandPredictor impl = SubsystemManager.GetSubsystem(); + Assert.Same(impl, predictor2); - ReadOnlyCollection impls = SubsystemManager.GetSubsystems(); - Assert.Single(impls); - Assert.Same(predictor2, impls[0]); + ReadOnlyCollection impls = SubsystemManager.GetSubsystems(); + Assert.Single(impls); + Assert.Same(predictor2, impls[0]); - // Unregister 'predictor2' - SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); + // Unregister 'predictor2' + SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); - VerifyCommandPredictorMetadata(ssInfo); - Assert.False(ssInfo.IsRegistered); - Assert.Empty(ssInfo.Implementations); + VerifyCommandPredictorMetadata(ssInfo); + Assert.False(ssInfo.IsRegistered); + Assert.Empty(ssInfo.Implementations); - impl = SubsystemManager.GetSubsystem(); - Assert.Null(impl); + impl = SubsystemManager.GetSubsystem(); + Assert.Null(impl); - impls = SubsystemManager.GetSubsystems(); - Assert.Empty(impls); + impls = SubsystemManager.GetSubsystems(); + Assert.Empty(impls); + } + finally + { + // Re-register the pre-existing predictors. + foreach (ICommandPredictor p in existingPredictors) + { + SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, p); + } + } } } } From ef957443f8ef7c54c2c4a006263f0959ca6e8255 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 11:53:47 -0700 Subject: [PATCH 19/29] Remove the debugging code --- test/xUnit/csharp/test_Subsystem.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index 473ff51f87b..594cebb4006 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -128,12 +128,6 @@ public static void GetSubsystemInfo() [Fact] public static void RegisterSubsystem() { - while (!System.Diagnostics.Debugger.IsAttached) - { - System.Threading.Thread.Sleep(200); - } - System.Diagnostics.Debugger.Break(); - // Unregister existing predictors, to make it easier to test. var existingPredictors = SubsystemManager.GetSubsystems(); foreach (ICommandPredictor p in existingPredictors) From e495d09bc6420ee92440c355a949b9fc0ac6b000 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 16:21:46 -0700 Subject: [PATCH 20/29] Make it an experimental feature --- .../host/msh/ConsoleHost.cs | 47 ++- .../ExperimentalFeature.cs | 4 + .../engine/Subsystem/SubsystemManager.cs | 7 +- .../engine/hostifaces/HostUtilities.cs | 389 ++++++++++++++++++ test/xUnit/csharp/test_Subsystem.cs | 138 ++----- 5 files changed, 485 insertions(+), 100 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 0a7cefe0ccf..9f1ecc10a30 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2500,7 +2500,14 @@ internal void Run(bool inputLoopIsNested) // Evaluate any suggestions if (previousResponseWasEmpty == false) { - EvaluateFeedbacks(ui); + if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSFeedbackProvider)) + { + EvaluateFeedbacks(ui); + } + else + { + EvaluateSuggestions(ui); + } } // Then output the prompt @@ -2860,6 +2867,44 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) } } + private void EvaluateSuggestions(ConsoleHostUserInterface ui) + { + // Output any training suggestions + try + { + List suggestions = HostUtilities.GetSuggestion(_parent.Runspace); + + if (suggestions.Count > 0) + { + ui.WriteLine(); + } + + bool first = true; + foreach (string suggestion in suggestions) + { + if (!first) + ui.WriteLine(); + + ui.WriteLine(suggestion); + + first = false; + } + } + catch (TerminateException) + { + // A variable breakpoint may be hit by HostUtilities.GetSuggestion. The debugger throws TerminateExceptions to stop the execution + // of the current statement; we do not want to treat these exceptions as errors. + } + catch (Exception e) + { + // Catch-all OK. This is a third-party call-out. + ui.WriteErrorLine(e.Message); + + LocalRunspace localRunspace = (LocalRunspace)_parent.Runspace; + localRunspace.GetExecutionContext.AppendDollarError(e); + } + } + private string EvaluatePrompt() { string promptString = _promptExec.ExecuteCommandAndGetResultAsString("prompt", out _); diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs index d83bd4bc8f3..5fe456f48d5 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs @@ -23,6 +23,7 @@ public class ExperimentalFeature internal const string EngineSource = "PSEngine"; internal const string PSNativeCommandErrorActionPreferenceFeatureName = "PSNativeCommandErrorActionPreference"; internal const string PSCustomTableHeaderLabelDecoration = "PSCustomTableHeaderLabelDecoration"; + internal const string PSFeedbackProvider = "PSFeedbackProvider"; #endregion @@ -120,6 +121,9 @@ static ExperimentalFeature() new ExperimentalFeature( name: PSCustomTableHeaderLabelDecoration, description: "Formatting differentiation for table header labels that aren't property members"), + new ExperimentalFeature( + name: PSFeedbackProvider, + description: "Replace the hard-coded suggestion framework with the extensible feedback provider"), }; EngineExperimentalFeatures = new ReadOnlyCollection(engineFeatures); diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index e4658bbdfad..3132a237a8d 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -57,12 +57,9 @@ static SubsystemManager() s_subSystemKindMap = new ReadOnlyDictionary(subSystemKindMap); // Register built-in suggestion providers. - RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSFeedbackProvider)) { - var instance = new UnixCommandNotFound(); - RegisterSubsystem(SubsystemKind.FeedbackProvider, instance); - RegisterSubsystem(SubsystemKind.CommandPredictor, instance); + RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); } } diff --git a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index 88f171794a0..ffbca7c117e 100644 --- a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs +++ b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs @@ -42,6 +42,64 @@ public static class HostUtilities { #region Internal Access + private static readonly string s_checkForCommandInCurrentDirectoryScript = @" + [System.Diagnostics.DebuggerHidden()] + param() + $foundSuggestion = $false + if($lastError -and + ($lastError.Exception -is ""System.Management.Automation.CommandNotFoundException"")) + { + $escapedCommand = [System.Management.Automation.WildcardPattern]::Escape($lastError.TargetObject) + $foundSuggestion = @(Get-Command ($ExecutionContext.SessionState.Path.Combine(""."", $escapedCommand)) -ErrorAction Ignore).Count -gt 0 + } + $foundSuggestion + "; + + private static readonly string s_createCommandExistsInCurrentDirectoryScript = @" + [System.Diagnostics.DebuggerHidden()] + param([string] $formatString) + $formatString -f $lastError.TargetObject,"".\$($lastError.TargetObject)"" + "; + + private static readonly string s_getFuzzyMatchedCommands = @" + [System.Diagnostics.DebuggerHidden()] + param([string] $formatString) + $formatString -f [string]::Join(', ', (Get-Command $lastError.TargetObject -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name)) + "; + + private static readonly List s_suggestions = InitializeSuggestions(); + + private static List InitializeSuggestions() + { + var suggestions = new List( + new Hashtable[] + { + NewSuggestion( + id: 3, + category: "General", + matchType: SuggestionMatchType.Dynamic, + rule: ScriptBlock.CreateDelayParsedScriptBlock(s_checkForCommandInCurrentDirectoryScript, isProductCode: true), + suggestion: ScriptBlock.CreateDelayParsedScriptBlock(s_createCommandExistsInCurrentDirectoryScript, isProductCode: true), + suggestionArgs: new object[] { CodeGeneration.EscapeSingleQuotedStringContent(SuggestionStrings.Suggestion_CommandExistsInCurrentDirectory) }, + enabled: true) + }); + + if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) + { + suggestions.Add( + NewSuggestion( + id: 4, + category: "General", + matchType: SuggestionMatchType.ErrorId, + rule: "CommandNotFoundException", + suggestion: ScriptBlock.CreateDelayParsedScriptBlock(s_getFuzzyMatchedCommands, isProductCode: true), + suggestionArgs: new object[] { CodeGeneration.EscapeSingleQuotedStringContent(SuggestionStrings.Suggestion_CommandNotFound) }, + enabled: true)); + } + + return suggestions; + } + #region GetProfileCommands /// /// Gets a PSObject whose base object is currentUserCurrentHost and with notes for the other 4 parameters. @@ -81,6 +139,42 @@ internal static void GetProfileObjectData(string shellId, bool useTestProfile, o dollarProfile = HostUtilities.GetDollarProfile(allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost); } + /// + /// Gets an array of commands that can be run sequentially to set $profile and run the profile commands. + /// + /// The id identifying the host or shell used in profile file names. + /// Used from test not to overwrite the profile file names from development boxes. + /// + internal static PSCommand[] GetProfileCommands(string shellId, bool useTestProfile) + { + List commands = new List(); + string allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost; + PSObject dollarProfile; + HostUtilities.GetProfileObjectData(shellId, useTestProfile, out allUsersAllHosts, out allUsersCurrentHost, out currentUserAllHosts, out currentUserCurrentHost, out dollarProfile); + + PSCommand command = new PSCommand(); + command.AddCommand("set-variable"); + command.AddParameter("Name", "profile"); + command.AddParameter("Value", dollarProfile); + command.AddParameter("Option", ScopedItemOptions.None); + commands.Add(command); + + string[] profilePaths = new string[] { allUsersAllHosts, allUsersCurrentHost, currentUserAllHosts, currentUserCurrentHost }; + foreach (string profilePath in profilePaths) + { + if (!System.IO.File.Exists(profilePath)) + { + continue; + } + + command = new PSCommand(); + command.AddCommand(profilePath, false); + commands.Add(command); + } + + return commands.ToArray(); + } + /// /// Used to get all profile file names for the current or all hosts and for the current or all users. /// @@ -184,6 +278,219 @@ internal static string GetMaxLines(string source, int maxLines) return returnValue.ToString(); } + internal static List GetSuggestion(Runspace runspace) + { + if (!(runspace is LocalRunspace localRunspace)) + { return new List(); } + + // Get the last value of $? + bool questionMarkVariableValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; + + // Get the last history item + History history = localRunspace.History; + HistoryInfo[] entries = history.GetEntries(-1, 1, true); + + if (entries.Length == 0) + return new List(); + + HistoryInfo lastHistory = entries[0]; + + // Get the last error + ArrayList errorList = (ArrayList)localRunspace.GetExecutionContext.DollarErrorVariable; + object lastError = null; + + if (errorList.Count > 0) + { + lastError = errorList[0] as Exception; + ErrorRecord lastErrorRecord = null; + + // The error was an actual ErrorRecord + if (lastError == null) + { + lastErrorRecord = errorList[0] as ErrorRecord; + } + else if (lastError is RuntimeException) + { + lastErrorRecord = ((RuntimeException)lastError).ErrorRecord; + } + + // If we got information about the error invocation, + // we can be more careful with the errors we pass along + if ((lastErrorRecord != null) && (lastErrorRecord.InvocationInfo != null)) + { + if (lastErrorRecord.InvocationInfo.HistoryId == lastHistory.Id) + lastError = lastErrorRecord; + else + lastError = null; + } + } + + Runspace oldDefault = null; + bool changedDefault = false; + if (Runspace.DefaultRunspace != runspace) + { + oldDefault = Runspace.DefaultRunspace; + changedDefault = true; + Runspace.DefaultRunspace = runspace; + } + + List suggestions = null; + + try + { + suggestions = GetSuggestion(lastHistory, lastError, errorList); + } + finally + { + if (changedDefault) + { + Runspace.DefaultRunspace = oldDefault; + } + } + + // Restore $? + localRunspace.ExecutionContext.QuestionMarkVariableValue = questionMarkVariableValue; + return suggestions; + } + + [SuppressMessage("Microsoft.Usage", "CA2208:InstantiateArgumentExceptionsCorrectly")] + internal static List GetSuggestion(HistoryInfo lastHistory, object lastError, ArrayList errorList) + { + var returnSuggestions = new List(); + + PSModuleInfo invocationModule = new PSModuleInfo(true); + invocationModule.SessionState.PSVariable.Set("lastHistory", lastHistory); + invocationModule.SessionState.PSVariable.Set("lastError", lastError); + + int initialErrorCount = 0; + + // Go through all of the suggestions + foreach (Hashtable suggestion in s_suggestions) + { + initialErrorCount = errorList.Count; + + // Make sure the rule is enabled + if (!LanguagePrimitives.IsTrue(suggestion["Enabled"])) + continue; + + SuggestionMatchType matchType = (SuggestionMatchType)LanguagePrimitives.ConvertTo( + suggestion["MatchType"], + typeof(SuggestionMatchType), + CultureInfo.InvariantCulture); + + // If this is a dynamic match, evaluate the ScriptBlock + if (matchType == SuggestionMatchType.Dynamic) + { + object result = null; + + ScriptBlock evaluator = suggestion["Rule"] as ScriptBlock; + if (evaluator == null) + { + suggestion["Enabled"] = false; + + throw new ArgumentException( + SuggestionStrings.RuleMustBeScriptBlock, "Rule"); + } + + try + { + result = invocationModule.Invoke(evaluator, null); + } + catch (Exception) + { + // Catch-all OK. This is a third-party call-out. + suggestion["Enabled"] = false; + continue; + } + + // If it returned results, evaluate its suggestion + if (LanguagePrimitives.IsTrue(result)) + { + string suggestionText = GetSuggestionText(suggestion["Suggestion"], (object[])suggestion["SuggestionArgs"], invocationModule); + + if (!string.IsNullOrEmpty(suggestionText)) + { + string returnString = string.Format( + CultureInfo.CurrentCulture, + "Suggestion [{0},{1}]: {2}", + (int)suggestion["Id"], + (string)suggestion["Category"], + suggestionText); + + returnSuggestions.Add(returnString); + } + } + } + else + { + string matchText = string.Empty; + + // Otherwise, this is a Regex match against the + // command or error + if (matchType == SuggestionMatchType.Command) + { + matchText = lastHistory.CommandLine; + } + else if (matchType == SuggestionMatchType.Error) + { + if (lastError != null) + { + Exception lastException = lastError as Exception; + if (lastException != null) + { + matchText = lastException.Message; + } + else + { + matchText = lastError.ToString(); + } + } + } + else if (matchType == SuggestionMatchType.ErrorId) + { + if (lastError != null && lastError is ErrorRecord errorRecord) + { + matchText = errorRecord.FullyQualifiedErrorId; + } + } + else + { + suggestion["Enabled"] = false; + + throw new ArgumentException( + SuggestionStrings.InvalidMatchType, + "MatchType"); + } + + // If the text matches, evaluate the suggestion + if (Regex.IsMatch(matchText, (string)suggestion["Rule"], RegexOptions.IgnoreCase)) + { + string suggestionText = GetSuggestionText(suggestion["Suggestion"], (object[])suggestion["SuggestionArgs"], invocationModule); + + if (!string.IsNullOrEmpty(suggestionText)) + { + string returnString = string.Format( + CultureInfo.CurrentCulture, + "Suggestion [{0},{1}]: {2}", + (int)suggestion["Id"], + (string)suggestion["Category"], + suggestionText); + + returnSuggestions.Add(returnString); + } + } + } + + // If the rule generated an error, disable it + if (errorList.Count != initialErrorCount) + { + suggestion["Enabled"] = false; + } + } + + return returnSuggestions; + } + /// /// Remove the GUID from the message if the message is in the pre-defined format. /// @@ -226,6 +533,88 @@ internal static string RemoveIdentifierInfoFromMessage(string message, out bool return message; } + /// + /// Create suggestion with string rule and scriptblock suggestion. + /// + /// Identifier for the suggestion. + /// Category for the suggestion. + /// Suggestion match type. + /// Rule to match. + /// Scriptblock to run that returns the suggestion. + /// Arguments to pass to suggestion scriptblock. + /// True if the suggestion is enabled. + /// Hashtable representing the suggestion. + private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, string rule, ScriptBlock suggestion, object[] suggestionArgs, bool enabled) + { + Hashtable result = new Hashtable(StringComparer.CurrentCultureIgnoreCase); + + result["Id"] = id; + result["Category"] = category; + result["MatchType"] = matchType; + result["Rule"] = rule; + result["Suggestion"] = suggestion; + result["SuggestionArgs"] = suggestionArgs; + result["Enabled"] = enabled; + + return result; + } + + /// + /// Create suggestion with scriptblock rule and suggestion. + /// + private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, ScriptBlock rule, ScriptBlock suggestion, bool enabled) + { + Hashtable result = new Hashtable(StringComparer.CurrentCultureIgnoreCase); + + result["Id"] = id; + result["Category"] = category; + result["MatchType"] = matchType; + result["Rule"] = rule; + result["Suggestion"] = suggestion; + result["Enabled"] = enabled; + + return result; + } + + /// + /// Create suggestion with scriptblock rule and scriptblock suggestion with arguments. + /// + private static Hashtable NewSuggestion(int id, string category, SuggestionMatchType matchType, ScriptBlock rule, ScriptBlock suggestion, object[] suggestionArgs, bool enabled) + { + Hashtable result = NewSuggestion(id, category, matchType, rule, suggestion, enabled); + result.Add("SuggestionArgs", suggestionArgs); + + return result; + } + + /// + /// Get suggestion text from suggestion scriptblock with arguments. + /// + private static string GetSuggestionText(object suggestion, object[] suggestionArgs, PSModuleInfo invocationModule) + { + if (suggestion is ScriptBlock) + { + ScriptBlock suggestionScript = (ScriptBlock)suggestion; + + object result = null; + try + { + result = invocationModule.Invoke(suggestionScript, suggestionArgs); + } + catch (Exception) + { + // Catch-all OK. This is a third-party call-out. + return string.Empty; + } + + return (string)LanguagePrimitives.ConvertTo(result, typeof(string), CultureInfo.CurrentCulture); + } + else + { + return (string)LanguagePrimitives.ConvertTo(suggestion, typeof(string), CultureInfo.CurrentCulture); + } + } + /// /// Returns the prompt used in remote sessions: "[machine]: basePrompt" /// diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index 594cebb4006..0d82880eb85 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -60,14 +60,20 @@ public static void GetSubsystemInfo() SubsystemInfo predictorInfo = SubsystemManager.GetSubsystemInfo(typeof(ICommandPredictor)); SubsystemInfo predictorInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); Assert.Same(predictorInfo2, predictorInfo); + VerifyCommandPredictorMetadata(predictorInfo); + Assert.False(predictorInfo.IsRegistered); + Assert.Empty(predictorInfo.Implementations); #endregion #region Feedback SubsystemInfo feedbackProviderInfo = SubsystemManager.GetSubsystemInfo(typeof(IFeedbackProvider)); SubsystemInfo feedback2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.FeedbackProvider); Assert.Same(feedback2, feedbackProviderInfo); + VerifyFeedbackProviderMetadata(feedbackProviderInfo); + Assert.True(feedbackProviderInfo.IsRegistered); + Assert.Single(feedbackProviderInfo.Implementations); #endregion #region DSC @@ -86,38 +92,13 @@ public static void GetSubsystemInfo() Assert.Same(ssInfos[1], crossPlatformDscInfo); Assert.Same(ssInfos[2], feedbackProviderInfo); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - Assert.True(predictorInfo.IsRegistered); - Assert.Single(predictorInfo.Implementations); - - Assert.True(feedbackProviderInfo.IsRegistered); - Assert.Equal(2, feedbackProviderInfo.Implementations.Count); - - ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); - Assert.NotNull(predictorImpl); - ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); - Assert.Single(predictorImpls); + ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); + Assert.Null(predictorImpl); + ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); + Assert.Empty(predictorImpls); - ReadOnlyCollection feedbackImpls = SubsystemManager.GetSubsystems(); - Assert.Equal(2, feedbackImpls.Count); - } - else - { - Assert.False(predictorInfo.IsRegistered); - Assert.Empty(predictorInfo.Implementations); - - Assert.True(feedbackProviderInfo.IsRegistered); - Assert.Single(feedbackProviderInfo.Implementations); - - ICommandPredictor predictorImpl = SubsystemManager.GetSubsystem(); - Assert.Null(predictorImpl); - ReadOnlyCollection predictorImpls = SubsystemManager.GetSubsystems(); - Assert.Empty(predictorImpls); - - ReadOnlyCollection feedbackImpls = SubsystemManager.GetSubsystems(); - Assert.Single(feedbackImpls); - } + ReadOnlyCollection feedbackImpls = SubsystemManager.GetSubsystems(); + Assert.Single(feedbackImpls); ICrossPlatformDsc crossPlatformDscImpl = SubsystemManager.GetSubsystem(); Assert.Null(crossPlatformDscImpl); @@ -128,13 +109,6 @@ public static void GetSubsystemInfo() [Fact] public static void RegisterSubsystem() { - // Unregister existing predictors, to make it easier to test. - var existingPredictors = SubsystemManager.GetSubsystems(); - foreach (ICommandPredictor p in existingPredictors) - { - SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, p.Id); - } - try { Assert.Throws( @@ -219,79 +193,55 @@ public static void RegisterSubsystem() { SubsystemManager.UnregisterSubsystem(predictor1.Id); SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); - - // Re-register the pre-existing predictors. - foreach (ICommandPredictor p in existingPredictors) - { - SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, p); - } } } [Fact] public static void UnregisterSubsystem() { - // Unregister existing predictors, to make it easier to test. - var existingPredictors = SubsystemManager.GetSubsystems(); - foreach (ICommandPredictor p in existingPredictors) - { - SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, p.Id); - } + // Exception expected when no implementation is registered + Assert.Throws(() => SubsystemManager.UnregisterSubsystem(predictor1.Id)); - try - { - // Exception expected when no implementation is registered - Assert.Throws(() => SubsystemManager.UnregisterSubsystem(predictor1.Id)); - - SubsystemManager.RegisterSubsystem(predictor1); - SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, predictor2); + SubsystemManager.RegisterSubsystem(predictor1); + SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, predictor2); - // Exception is expected when specified id cannot be found - Assert.Throws(() => SubsystemManager.UnregisterSubsystem(Guid.NewGuid())); + // Exception is expected when specified id cannot be found + Assert.Throws(() => SubsystemManager.UnregisterSubsystem(Guid.NewGuid())); - // Unregister 'predictor1' - SubsystemManager.UnregisterSubsystem(predictor1.Id); + // Unregister 'predictor1' + SubsystemManager.UnregisterSubsystem(predictor1.Id); - SubsystemInfo ssInfo = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); - VerifyCommandPredictorMetadata(ssInfo); - Assert.True(ssInfo.IsRegistered); - Assert.Single(ssInfo.Implementations); + SubsystemInfo ssInfo = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); + VerifyCommandPredictorMetadata(ssInfo); + Assert.True(ssInfo.IsRegistered); + Assert.Single(ssInfo.Implementations); - var implInfo = ssInfo.Implementations[0]; - Assert.Equal(predictor2.Id, implInfo.Id); - Assert.Equal(predictor2.Name, implInfo.Name); - Assert.Equal(predictor2.Description, implInfo.Description); - Assert.Equal(SubsystemKind.CommandPredictor, implInfo.Kind); - Assert.Same(typeof(MyPredictor), implInfo.ImplementationType); + var implInfo = ssInfo.Implementations[0]; + Assert.Equal(predictor2.Id, implInfo.Id); + Assert.Equal(predictor2.Name, implInfo.Name); + Assert.Equal(predictor2.Description, implInfo.Description); + Assert.Equal(SubsystemKind.CommandPredictor, implInfo.Kind); + Assert.Same(typeof(MyPredictor), implInfo.ImplementationType); - ICommandPredictor impl = SubsystemManager.GetSubsystem(); - Assert.Same(impl, predictor2); + ICommandPredictor impl = SubsystemManager.GetSubsystem(); + Assert.Same(impl, predictor2); - ReadOnlyCollection impls = SubsystemManager.GetSubsystems(); - Assert.Single(impls); - Assert.Same(predictor2, impls[0]); + ReadOnlyCollection impls = SubsystemManager.GetSubsystems(); + Assert.Single(impls); + Assert.Same(predictor2, impls[0]); - // Unregister 'predictor2' - SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); + // Unregister 'predictor2' + SubsystemManager.UnregisterSubsystem(SubsystemKind.CommandPredictor, predictor2.Id); - VerifyCommandPredictorMetadata(ssInfo); - Assert.False(ssInfo.IsRegistered); - Assert.Empty(ssInfo.Implementations); + VerifyCommandPredictorMetadata(ssInfo); + Assert.False(ssInfo.IsRegistered); + Assert.Empty(ssInfo.Implementations); - impl = SubsystemManager.GetSubsystem(); - Assert.Null(impl); + impl = SubsystemManager.GetSubsystem(); + Assert.Null(impl); - impls = SubsystemManager.GetSubsystems(); - Assert.Empty(impls); - } - finally - { - // Re-register the pre-existing predictors. - foreach (ICommandPredictor p in existingPredictors) - { - SubsystemManager.RegisterSubsystem(SubsystemKind.CommandPredictor, p); - } - } + impls = SubsystemManager.GetSubsystems(); + Assert.Empty(impls); } } } From baa1b1726959e0866824154d4260849ece4000e4 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 16:26:04 -0700 Subject: [PATCH 21/29] Fix a few code factor issues --- .../engine/Subsystem/SubsystemManager.cs | 3 +-- test/xUnit/csharp/test_Subsystem.cs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index 3132a237a8d..de5dbe9e354 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -8,9 +8,8 @@ using System.Collections.ObjectModel; using System.Management.Automation.Internal; using System.Management.Automation.Subsystem.DSC; -using System.Management.Automation.Subsystem.Prediction; using System.Management.Automation.Subsystem.Feedback; -using System.Runtime.InteropServices; +using System.Management.Automation.Subsystem.Prediction; namespace System.Management.Automation.Subsystem { diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index 0d82880eb85..11ac9d55419 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -5,10 +5,9 @@ using System.Collections.ObjectModel; using System.Management.Automation.Subsystem; using System.Management.Automation.Subsystem.DSC; -using System.Management.Automation.Subsystem.Prediction; using System.Management.Automation.Subsystem.Feedback; +using System.Management.Automation.Subsystem.Prediction; using System.Threading; -using System.Runtime.InteropServices; using Xunit; namespace PSTests.Sequential From 8dba6f217ff85710e3ebc8ad64712f98ce213e25 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 16:33:44 -0700 Subject: [PATCH 22/29] Revert some style changes to HostUtilities.cs --- .../engine/hostifaces/HostUtilities.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index ffbca7c117e..01bdf8ea3c8 100644 --- a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs +++ b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs @@ -45,25 +45,30 @@ public static class HostUtilities private static readonly string s_checkForCommandInCurrentDirectoryScript = @" [System.Diagnostics.DebuggerHidden()] param() + $foundSuggestion = $false + if($lastError -and ($lastError.Exception -is ""System.Management.Automation.CommandNotFoundException"")) { $escapedCommand = [System.Management.Automation.WildcardPattern]::Escape($lastError.TargetObject) $foundSuggestion = @(Get-Command ($ExecutionContext.SessionState.Path.Combine(""."", $escapedCommand)) -ErrorAction Ignore).Count -gt 0 } + $foundSuggestion "; private static readonly string s_createCommandExistsInCurrentDirectoryScript = @" [System.Diagnostics.DebuggerHidden()] param([string] $formatString) + $formatString -f $lastError.TargetObject,"".\$($lastError.TargetObject)"" "; private static readonly string s_getFuzzyMatchedCommands = @" [System.Diagnostics.DebuggerHidden()] param([string] $formatString) + $formatString -f [string]::Join(', ', (Get-Command $lastError.TargetObject -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name)) "; @@ -280,8 +285,7 @@ internal static string GetMaxLines(string source, int maxLines) internal static List GetSuggestion(Runspace runspace) { - if (!(runspace is LocalRunspace localRunspace)) - { return new List(); } + if (!(runspace is LocalRunspace localRunspace)) { return new List(); } // Get the last value of $? bool questionMarkVariableValue = localRunspace.ExecutionContext.QuestionMarkVariableValue; From 6cddafaf5121eb0f2c99cdb5be8c22990049e3f0 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Oct 2022 17:04:23 -0700 Subject: [PATCH 23/29] Update the experimental json file --- experimental-feature-linux.json | 3 ++- experimental-feature-windows.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/experimental-feature-linux.json b/experimental-feature-linux.json index 573cfc1084a..08f398171d4 100644 --- a/experimental-feature-linux.json +++ b/experimental-feature-linux.json @@ -3,5 +3,6 @@ "PSCustomTableHeaderLabelDecoration", "PSLoadAssemblyFromNativeCode", "PSNativeCommandErrorActionPreference", - "PSSubsystemPluginModel" + "PSSubsystemPluginModel", + "PSFeedbackProvider" ] diff --git a/experimental-feature-windows.json b/experimental-feature-windows.json index 573cfc1084a..08f398171d4 100644 --- a/experimental-feature-windows.json +++ b/experimental-feature-windows.json @@ -3,5 +3,6 @@ "PSCustomTableHeaderLabelDecoration", "PSLoadAssemblyFromNativeCode", "PSNativeCommandErrorActionPreference", - "PSSubsystemPluginModel" + "PSSubsystemPluginModel", + "PSFeedbackProvider" ] From 55fcf94be99e0ff6ee58018b50d83dc8f04cbe68 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 12 Oct 2022 10:26:59 -0700 Subject: [PATCH 24/29] Always register the general feedback provider --- .../engine/Subsystem/SubsystemManager.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs index de5dbe9e354..501bebcaddc 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -56,10 +56,7 @@ static SubsystemManager() s_subSystemKindMap = new ReadOnlyDictionary(subSystemKindMap); // Register built-in suggestion providers. - if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSFeedbackProvider)) - { - RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); - } + RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); } #region internal - Retrieve subsystem proxy object From a684a37383dc49569c64fb3953977bcfc3bcc7fc Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 12 Oct 2022 11:32:51 -0700 Subject: [PATCH 25/29] Add feedback styles to PSStyle.Formatting --- .../host/msh/ConsoleHost.cs | 13 ++++++++--- .../PowerShellCore_format_ps1xml.cs | 10 ++++++-- .../FormatAndOutput/common/PSStyle.cs | 23 +++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 9f1ecc10a30..6fb0ad51cf8 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2825,6 +2825,10 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) ui.WriteLine(); const string Indentation = " "; + string nameStyle = PSStyle.Instance.Formatting.FeedbackProvider; + string textStyle = PSStyle.Instance.Formatting.FeedbackText; + string ansiReset = PSStyle.Instance.Reset; + int count = 0; var output = new StringBuilder(); @@ -2836,10 +2840,11 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) } output.Append("Suggestion [") - .Append(PSStyle.Instance.Foreground.BrightGreen) + .Append(nameStyle) .Append(entry.Name) - .Append(PSStyle.Instance.Reset) - .AppendLine("]:"); + .Append(ansiReset) + .AppendLine("]:") + .Append(textStyle); string[] lines = entry.Text.Split('\n', StringSplitOptions.RemoveEmptyEntries); foreach (string line in lines) @@ -2849,7 +2854,9 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) .AppendLine(); } + output.Append(ansiReset); ui.Write(output.ToString()); + count++; output.Clear(); } diff --git a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs index 9b2f6596bd5..80aef760ac5 100644 --- a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs +++ b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs @@ -2030,12 +2030,15 @@ private static IEnumerable ViewsOf_System_Management_Autom .AddItemScriptBlock(@"""$($_.Strikethrough)$($_.Strikethrough.Replace(""""`e"""",'`e'))$($_.Reset)""", label: "Strikethrough") .AddItemProperty(@"OutputRendering") .AddItemScriptBlock(@"""$($_.Formatting.FormatAccent)$($_.Formatting.FormatAccent.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FormatAccent") - .AddItemScriptBlock(@"""$($_.Formatting.TableHeader)$($_.Formatting.TableHeader.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.TableHeader") .AddItemScriptBlock(@"""$($_.Formatting.ErrorAccent)$($_.Formatting.ErrorAccent.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.ErrorAccent") .AddItemScriptBlock(@"""$($_.Formatting.Error)$($_.Formatting.Error.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.Error") .AddItemScriptBlock(@"""$($_.Formatting.Warning)$($_.Formatting.Warning.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.Warning") .AddItemScriptBlock(@"""$($_.Formatting.Verbose)$($_.Formatting.Verbose.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.Verbose") .AddItemScriptBlock(@"""$($_.Formatting.Debug)$($_.Formatting.Debug.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.Debug") + .AddItemScriptBlock(@"""$($_.Formatting.TableHeader)$($_.Formatting.TableHeader.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.TableHeader") + .AddItemScriptBlock(@"""$($_.Formatting.CustomTableHeaderLabel)$($_.Formatting.CustomTableHeaderLabel.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.CustomTableHeaderLabel") + .AddItemScriptBlock(@"""$($_.Formatting.FeedbackProvider)$($_.Formatting.FeedbackProvider.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FeedbackProvider") + .AddItemScriptBlock(@"""$($_.Formatting.FeedbackText)$($_.Formatting.FeedbackText.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FeedbackText") .AddItemScriptBlock(@"""$($_.Progress.Style)$($_.Progress.Style.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Progress.Style") .AddItemScriptBlock(@"""$($_.Progress.MaxWidth)""", label: "Progress.MaxWidth") .AddItemScriptBlock(@"""$($_.Progress.View)""", label: "Progress.View") @@ -2086,12 +2089,15 @@ private static IEnumerable ViewsOf_System_Management_Autom ListControl.Create() .StartEntry() .AddItemScriptBlock(@"""$($_.FormatAccent)$($_.FormatAccent.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FormatAccent") - .AddItemScriptBlock(@"""$($_.TableHeader)$($_.TableHeader.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "TableHeader") .AddItemScriptBlock(@"""$($_.ErrorAccent)$($_.ErrorAccent.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "ErrorAccent") .AddItemScriptBlock(@"""$($_.Error)$($_.Error.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Error") .AddItemScriptBlock(@"""$($_.Warning)$($_.Warning.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Warning") .AddItemScriptBlock(@"""$($_.Verbose)$($_.Verbose.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Verbose") .AddItemScriptBlock(@"""$($_.Debug)$($_.Debug.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Debug") + .AddItemScriptBlock(@"""$($_.TableHeader)$($_.TableHeader.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "TableHeader") + .AddItemScriptBlock(@"""$($_.CustomTableHeaderLabel)$($_.CustomTableHeaderLabel.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "CustomTableHeaderLabel") + .AddItemScriptBlock(@"""$($_.FeedbackProvider)$($_.FeedbackProvider.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FeedbackProvider") + .AddItemScriptBlock(@"""$($_.FeedbackText)$($_.FeedbackText.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FeedbackText") .EndEntry() .EndList()); } diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index b6e7b9370e1..bdfb9da752b 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Collections.Generic; using System.Management.Automation.Internal; +using System.Security.Policy; namespace System.Management.Automation { @@ -431,6 +432,28 @@ public string Debug } private string _debug = "\x1b[33;1m"; + + /// + /// Gets or sets the style for rendering feedback provider names. + /// + public string FeedbackProvider + { + get => _feedbackProvider; + set => _feedbackProvider = ValidateNoContent(value); + } + + private string _feedbackProvider = "\x1b[33m"; + + /// + /// Gets or sets the style for rendering feedback text. + /// + public string FeedbackText + { + get => _feedbackText; + set => _feedbackText = ValidateNoContent(value); + } + + private string _feedbackText = "\x1b[96m"; } /// From 7a07d12e725cda3c3d62f5913139145c43f98973 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 13 Oct 2022 14:35:53 -0700 Subject: [PATCH 26/29] Use '-FuzzyMinimumDistance' --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 2 +- .../engine/hostifaces/HostUtilities.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index 85e03df40e2..aff98b46019 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -88,7 +88,7 @@ internal GeneralCommandErrorFeedback() { _fuzzySb ??= ScriptBlock.CreateDelayParsedScriptBlock(@$" param([string] $target) - $cmdNames = Get-Command $target -UseFuzzyMatch | Select-Object -First 10 -ExpandProperty Name + $cmdNames = Get-Command $target -UseFuzzyMatching -FuzzyMinimumDistance 1 | Select-Object -First 5 -Unique -ExpandProperty Name if ($cmdNames) {{ [string]::Join(', ', $cmdNames) }} diff --git a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index 01bdf8ea3c8..c543ad11f13 100644 --- a/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs +++ b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs @@ -69,7 +69,7 @@ public static class HostUtilities [System.Diagnostics.DebuggerHidden()] param([string] $formatString) - $formatString -f [string]::Join(', ', (Get-Command $lastError.TargetObject -UseFuzzyMatch | Select-Object -First 10 -Unique -ExpandProperty Name)) + $formatString -f [string]::Join(', ', (Get-Command $lastError.TargetObject -UseFuzzyMatching -FuzzyMinimumDistance 1 | Select-Object -First 5 -Unique -ExpandProperty Name)) "; private static readonly List s_suggestions = InitializeSuggestions(); From b0d9183be8a15552b6c07fc12407d99fde7dffbd Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 17 Oct 2022 14:09:37 -0700 Subject: [PATCH 27/29] Add test for feedback provider interface --- .../host/msh/ConsoleHost.cs | 7 + test/xUnit/csharp/test_Feedback.cs | 120 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 test/xUnit/csharp/test_Feedback.cs diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 6fb0ad51cf8..ab2698ab7ec 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -2829,6 +2829,13 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui) string textStyle = PSStyle.Instance.Formatting.FeedbackText; string ansiReset = PSStyle.Instance.Reset; + if (!ui.SupportsVirtualTerminal) + { + nameStyle = string.Empty; + textStyle = string.Empty; + ansiReset = string.Empty; + } + int count = 0; var output = new StringBuilder(); diff --git a/test/xUnit/csharp/test_Feedback.cs b/test/xUnit/csharp/test_Feedback.cs new file mode 100644 index 00000000000..d57d21fa0ec --- /dev/null +++ b/test/xUnit/csharp/test_Feedback.cs @@ -0,0 +1,120 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.IO; +using System.Management.Automation; +using System.Management.Automation.Subsystem; +using System.Management.Automation.Subsystem.Feedback; +using System.Threading; +using Xunit; + +namespace PSTests.Sequential +{ + public class MyFeedback : IFeedbackProvider + { + private readonly Guid _id; + private readonly string _name, _description; + private readonly bool _delay; + + public static readonly MyFeedback SlowFeedback; + + static MyFeedback() + { + SlowFeedback = new MyFeedback( + Guid.NewGuid(), + "Slow", + "Description for #1 feedback provider.", + delay: true); + } + + private MyFeedback(Guid id, string name, string description, bool delay) + { + _id = id; + _name = name; + _description = description; + _delay = delay; + } + + public Guid Id => _id; + + public string Name => _name; + + public string Description => _description; + + public string GetFeedback(string commandLine, ErrorRecord errorRecord, CancellationToken token) + { + if (_delay) + { + // The delay 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. + Thread.Sleep(2000); + } + + return $"{commandLine}+{errorRecord.FullyQualifiedErrorId}"; + } + } + + public static class FeedbackProviderTests + { + [Fact] + public static void GetFeedback() + { + var pwsh = PowerShell.Create(); + var settings = new PSInvocationSettings() { AddToHistory = true }; + + // Setup the context for the test. + // Change current working directory to the temp path. + pwsh.AddCommand("Set-Location") + .AddParameter("Path", Path.GetTempPath()) + .Invoke(input: null, settings); + pwsh.Commands.Clear(); + + // Create an empty file 'feedbacktest.ps1' under the temp path; + pwsh.AddCommand("New-Item") + .AddParameter("Path", "feedbacktest.ps1") + .Invoke(input: null, settings); + pwsh.Commands.Clear(); + + // Run a command 'feedbacktest', so as to trigger the 'General' feedback. + pwsh.AddScript("feedbacktest").Invoke(input: null, settings); + pwsh.Commands.Clear(); + + try + { + // Register the slow feedback provider. + // The 'General' feedback provider is built-in and registered by default. + SubsystemManager.RegisterSubsystem(SubsystemKind.FeedbackProvider, MyFeedback.SlowFeedback); + + // Expect the result from 'General' only because the 'slow' one 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. + var feedbacks = FeedbackHub.GetFeedback(pwsh.Runspace, millisecondsTimeout: 1000); + string expectedCmd = Path.Combine(".", "feedbacktest"); + + // Test the result from the 'General' feedback provider. + Assert.Single(feedbacks); + Assert.Equal("General", feedbacks[0].Name); + Assert.Contains(expectedCmd, feedbacks[0].Text); + + // Expect the result from both 'General' and the 'slow' feedback providers. + // 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. + feedbacks = FeedbackHub.GetFeedback(pwsh.Runspace, millisecondsTimeout: 4000); + Assert.Equal(2, feedbacks.Count); + + FeedbackEntry entry1 = feedbacks[0]; + Assert.Equal("General", entry1.Name); + Assert.Contains(expectedCmd, entry1.Text); + + FeedbackEntry entry2 = feedbacks[1]; + Assert.Equal("Slow", entry2.Name); + Assert.Equal("feedbacktest+CommandNotFoundException", entry2.Text); + } + finally + { + SubsystemManager.UnregisterSubsystem(MyFeedback.SlowFeedback.Id); + } + } + } +} From c060dbb654269772d696df090207b22f7d6d3343 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 17 Oct 2022 14:15:06 -0700 Subject: [PATCH 28/29] Address feedback --- .../engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs index aff98b46019..a254cd9b013 100644 --- a/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -224,7 +224,7 @@ public SuggestionPackage GetSuggestion(PredictionClient client, PredictionContex { // The line is a candidate if it starts with "sudo ", such as "sudo apt install python3". // 'sudo' is a command name that remains the same, so this check should work for all locales. - bool isCandidate = text.StartsWith("sudo "); + bool isCandidate = text.StartsWith("sudo ", StringComparison.Ordinal); int index = text.IndexOf('\n'); if (isCandidate) { From 50102aa8bc50ae7592346edf4927f1da43e28e8c Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 17 Oct 2022 14:22:32 -0700 Subject: [PATCH 29/29] Minor fixes --- .../FormatAndOutput/common/PSStyle.cs | 1 - .../resources/SuggestionStrings.resx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs index bdfb9da752b..ec3955c29bc 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -4,7 +4,6 @@ using System.Collections; using System.Collections.Generic; using System.Management.Automation.Internal; -using System.Security.Policy; namespace System.Management.Automation { diff --git a/src/System.Management.Automation/resources/SuggestionStrings.resx b/src/System.Management.Automation/resources/SuggestionStrings.resx index 7a8dabf2958..14e9c01b275 100644 --- a/src/System.Management.Automation/resources/SuggestionStrings.resx +++ b/src/System.Management.Automation/resources/SuggestionStrings.resx @@ -118,7 +118,7 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - The command {0} was not found, but does exist in the current location. PowerShell does not load commands from the current location by default. If you trust this command, instead type: "{1}". See "get-help about_Command_Precedence" for more details. + The command "{0}" was not found, but does exist in the current location. PowerShell does not load commands from the current location by default. If you trust this command, instead type: "{1}". See "get-help about_Command_Precedence" for more details. The most similar commands are: {0}.