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" ] diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index 812f42cfd5e..ab2698ab7ec 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.Feedback; using System.Management.Automation.Tracing; using System.Reflection; using System.Runtime; @@ -2489,7 +2488,6 @@ internal void Run(bool inputLoopIsNested) if (inBlockMode) { // use a special prompt that denotes block mode - prompt = ">> "; } else @@ -2502,7 +2500,14 @@ internal void Run(bool inputLoopIsNested) // Evaluate any suggestions if (previousResponseWasEmpty == false) { - EvaluateSuggestions(ui); + if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSFeedbackProvider)) + { + EvaluateFeedbacks(ui); + } + else + { + EvaluateSuggestions(ui); + } } // Then output the prompt @@ -2805,6 +2810,77 @@ private static bool IsIncompleteParseException(Exception e) return remoteException.ErrorRecord.CategoryInfo.Reason == nameof(IncompleteParseException); } + private void EvaluateFeedbacks(ConsoleHostUserInterface ui) + { + // Output any training suggestions + try + { + List feedbacks = FeedbackHub.GetFeedback(_parent.Runspace); + if (feedbacks is null || feedbacks.Count == 0) + { + return; + } + + // Feedback section starts with a new line. + ui.WriteLine(); + + const string Indentation = " "; + string nameStyle = PSStyle.Instance.Formatting.FeedbackProvider; + 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(); + + foreach (FeedbackEntry entry in feedbacks) + { + if (count > 0) + { + output.AppendLine(); + } + + output.Append("Suggestion [") + .Append(nameStyle) + .Append(entry.Name) + .Append(ansiReset) + .AppendLine("]:") + .Append(textStyle); + + string[] lines = entry.Text.Split('\n', StringSplitOptions.RemoveEmptyEntries); + foreach (string line in lines) + { + output.Append(Indentation) + .Append(line.AsSpan().TrimEnd()) + .AppendLine(); + } + + output.Append(ansiReset); + ui.Write(output.ToString()); + + count++; + output.Clear(); + } + + // Feedback section ends with a new line. + ui.WriteLine(); + } + 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 void EvaluateSuggestions(ConsoleHostUserInterface ui) { // Output any training suggestions 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..ec3955c29bc 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs @@ -431,6 +431,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"; } /// 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/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 new file mode 100644 index 00000000000..a254cd9b013 --- /dev/null +++ b/src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/IFeedbackProvider.cs @@ -0,0 +1,285 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#nullable enable + +using System; +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; + +namespace System.Management.Automation.Subsystem.Feedback +{ + /// + /// Interface for implementing a feedback provider on command failures. + /// + public interface IFeedbackProvider : ISubsystem + { + /// + /// Default implementation. No function is required for a feedback provider. + /// + Dictionary? ISubsystem.FunctionsToDefine => null; + + /// + /// Default implementation for `ISubsystem.Kind`. + /// + SubsystemKind ISubsystem.Kind => SubsystemKind.FeedbackProvider; + + /// + /// Gets feedback based on the given commandline and error record. + /// + /// + string? GetFeedback(string commandLine, ErrorRecord lastError, CancellationToken token); + } + + 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; + + public string Name => "General"; + + 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; + if (rsToUse is null) + { + return null; + } + + if (lastError.FullyQualifiedErrorId == "CommandNotFoundException") + { + EngineIntrinsics context = rsToUse.ExecutionContext.EngineIntrinsics; + + var target = (string)lastError.TargetObject; + CommandInvocationIntrinsics invocation = context.SessionState.InvokeCommand; + + // 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); + } + + // Check fuzzy matching command names. + if (ExperimentalFeature.IsEnabled("PSCommandNotFoundSuggestion")) + { + _fuzzySb ??= ScriptBlock.CreateDelayParsedScriptBlock(@$" + param([string] $target) + $cmdNames = Get-Command $target -UseFuzzyMatching -FuzzyMinimumDistance 1 | Select-Object -First 5 -Unique -ExpandProperty Name + if ($cmdNames) {{ + [string]::Join(', ', $cmdNames) + }} + ", isProductCode: true); + + _args[0] = target; + var result = _fuzzySb.InvokeReturnAsIs(_args); + + if (result is not null && result != AutomationNull.Value) + { + return StringUtil.Format( + SuggestionStrings.Suggestion_CommandNotFound, + result.ToString()); + } + } + } + + return null; + } + } + + 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/prediction source for the Unix command utility."; + + #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) + { + if (Platform.IsWindows || lastError.FullyQualifiedErrorId != "CommandNotFoundException") + { + return null; + } + + var target = (string)lastError.TargetObject; + if (target is null) + { + return null; + } + + if (target.EndsWith(".ps1", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + string? cmd_not_found = GetUtilityPath(); + if (cmd_not_found is not null) + { + var startInfo = new ProcessStartInfo(cmd_not_found); + startInfo.ArgumentList.Add(target); + startInfo.RedirectStandardError = true; + startInfo.RedirectStandardOutput = true; + + using var process = Process.Start(startInfo); + var stderr = process?.StandardError.ReadToEnd().Trim(); + + // The feedback contains recommended actions only if the output has multiple lines of text. + if (stderr?.IndexOf('\n') > 0) + { + _notFoundFeedback = stderr; + + var stdout = process?.StandardOutput.ReadToEnd().Trim(); + return string.IsNullOrEmpty(stdout) ? stderr : $"{stderr}\n{stdout}"; + } + } + + 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) + { + if (_candidates is null && _notFoundFeedback is not null) + { + var text = _notFoundFeedback.AsSpan(); + // Set to null to avoid potential race condition. + _notFoundFeedback = null; + + // This loop searches for candidate results with almost no allocation. + while (true) + { + // 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 ", StringComparison.Ordinal); + int index = text.IndexOf('\n'); + if (isCandidate) + { + 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); + } + } + + 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(result); + } + } + + 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 db842f435bb..e8937486d41 100644 --- a/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs +++ b/src/System.Management.Automation/engine/Subsystem/ISubsystem.cs @@ -10,8 +10,10 @@ namespace System.Management.Automation.Subsystem { /// /// Define the kinds of subsystems. + /// Allow composite enum values to enable one subsystem implementation to serve as multiple subystems. /// - public enum SubsystemKind + [Flags] + public enum SubsystemKind : uint { /// /// Component that provides predictive suggestions to commandline input. @@ -22,6 +24,11 @@ public enum SubsystemKind /// Cross platform desired state configuration component. /// CrossPlatformDsc = 2, + + /// + /// Component that provides feedback when a command fails interactively. + /// + FeedbackProvider = 4, } /// 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 diff --git a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs index ae555e8d187..e8d8fe39eae 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemInfo.cs @@ -78,6 +78,8 @@ public abstract class SubsystemInfo private protected SubsystemInfo(SubsystemKind kind, Type subsystemType) { + 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 d603436790c..501bebcaddc 100644 --- a/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs +++ b/src/System.Management.Automation/engine/Subsystem/SubsystemManager.cs @@ -6,9 +6,9 @@ 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.Feedback; using System.Management.Automation.Subsystem.Prediction; namespace System.Management.Automation.Subsystem @@ -35,6 +35,11 @@ static SubsystemManager() SubsystemKind.CrossPlatformDsc, allowUnregistration: true, allowMultipleRegistration: false), + + SubsystemInfo.Create( + SubsystemKind.FeedbackProvider, + allowUnregistration: true, + allowMultipleRegistration: true), }; var subSystemTypeMap = new Dictionary(subsystems.Length); @@ -49,6 +54,9 @@ static SubsystemManager() s_subsystems = new ReadOnlyCollection(subsystems); s_subSystemTypeMap = new ReadOnlyDictionary(subSystemTypeMap); s_subSystemKindMap = new ReadOnlyDictionary(subSystemKindMap); + + // Register built-in suggestion providers. + RegisterSubsystem(SubsystemKind.FeedbackProvider, new GeneralCommandErrorFeedback()); } #region internal - Retrieve subsystem proxy object @@ -143,6 +151,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; @@ -181,8 +191,9 @@ public static void RegisterSubsystem(TImple public static void RegisterSubsystem(SubsystemKind kind, ISubsystem proxy) { Requires.NotNull(proxy, nameof(proxy)); + Requires.OneSpecificSubsystemKind(kind); - if (kind != proxy.Kind) + if (!proxy.Kind.HasFlag(kind)) { throw new ArgumentException( StringUtil.Format( @@ -267,6 +278,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/engine/hostifaces/HostUtilities.cs b/src/System.Management.Automation/engine/hostifaces/HostUtilities.cs index b3e29423a83..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(); @@ -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", @@ -138,16 +124,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. /// @@ -561,30 +537,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 +591,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 +645,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/SubsystemStrings.resx b/src/System.Management.Automation/resources/SubsystemStrings.resx index a2819da8682..1ca0453ae31 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 here should represent one specific subsystem, but the given value is either invalid or a composite enum value: '{0}'. + diff --git a/src/System.Management.Automation/resources/SuggestionStrings.resx b/src/System.Management.Automation/resources/SuggestionStrings.resx index cd44c33e759..14e9c01b275 100644 --- a/src/System.Management.Automation/resources/SuggestionStrings.resx +++ b/src/System.Management.Automation/resources/SuggestionStrings.resx @@ -117,14 +117,8 @@ 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. + 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}. 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); + } + } + } +} diff --git a/test/xUnit/csharp/test_Subsystem.cs b/test/xUnit/csharp/test_Subsystem.cs index d5bc3b9c728..11ac9d55419 100644 --- a/test/xUnit/csharp/test_Subsystem.cs +++ b/test/xUnit/csharp/test_Subsystem.cs @@ -5,6 +5,7 @@ using System.Collections.ObjectModel; using System.Management.Automation.Subsystem; using System.Management.Automation.Subsystem.DSC; +using System.Management.Automation.Subsystem.Feedback; using System.Management.Automation.Subsystem.Prediction; using System.Threading; using Xunit; @@ -41,37 +42,63 @@ 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)); + SubsystemInfo predictorInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); + Assert.Same(predictorInfo2, predictorInfo); VerifyCommandPredictorMetadata(predictorInfo); Assert.False(predictorInfo.IsRegistered); Assert.Empty(predictorInfo.Implementations); + #endregion - SubsystemInfo predictorInfo2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.CommandPredictor); - Assert.Same(predictorInfo2, predictorInfo); + #region Feedback + SubsystemInfo feedbackProviderInfo = SubsystemManager.GetSubsystemInfo(typeof(IFeedbackProvider)); + SubsystemInfo feedback2 = SubsystemManager.GetSubsystemInfo(SubsystemKind.FeedbackProvider); + Assert.Same(feedback2, feedbackProviderInfo); - SubsystemInfo crossPlatformDscInfo = SubsystemManager.GetSubsystemInfo(typeof(ICrossPlatformDsc)); + VerifyFeedbackProviderMetadata(feedbackProviderInfo); + Assert.True(feedbackProviderInfo.IsRegistered); + Assert.Single(feedbackProviderInfo.Implementations); + #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); 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); ReadOnlyCollection crossPlatformDscImpls = SubsystemManager.GetSubsystems(); @@ -91,14 +118,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);