Skip to content

Update the FeedbackProvider interface to return structured data #19133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daxian-dbw merged 10 commits into from
Feb 23, 2023
Merged

Update the FeedbackProvider interface to return structured data #19133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
221988d
Update feedback interface to return structured data
daxian-dbw Feb 10, 2023
eca79a4
Minor update to 'RenderActions' to remove extra line at end when deal…
daxian-dbw Feb 10, 2023
4e62aed
Refactor 'RenderFeedback' method
daxian-dbw Feb 10, 2023
651c8ae
Add a comment
daxian-dbw Feb 10, 2023
08756c7
Fix up the tests
daxian-dbw Feb 11, 2023
848befe
Fix CodeFactor issues
daxian-dbw Feb 11, 2023
64399fb
Address Ilya's comments
daxian-dbw Feb 14, 2023
5b1e477
Address Patrick's feedback
daxian-dbw Feb 17, 2023
eb7f449
Address some more feedback
daxian-dbw Feb 17, 2023
d87f2d4
Avoid exposing public nested type
daxian-dbw Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 3 additions & 51 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2836,61 +2836,13 @@ private void EvaluateFeedbacks(ConsoleHostUserInterface ui)
// Output any training suggestions
try
{
List<FeedbackEntry> feedbacks = FeedbackHub.GetFeedback(_parent.Runspace);
if (feedbacks is null || feedbacks.Count == 0)
List<FeedbackResult> feedbacks = FeedbackHub.GetFeedback(_parent.Runspace);
if (feedbacks is null || feedbacks.Count is 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();
HostUtilities.RenderFeedback(feedbacks, ui);
}
catch (Exception e)
{
Expand Down
6 changes: 4 additions & 2 deletions ...m.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2063,8 +2063,9 @@ private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Autom
.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.FeedbackName)$($_.Formatting.FeedbackName.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FeedbackName")
.AddItemScriptBlock(@"""$($_.Formatting.FeedbackText)$($_.Formatting.FeedbackText.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FeedbackText")
.AddItemScriptBlock(@"""$($_.Formatting.FeedbackAction)$($_.Formatting.FeedbackAction.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "Formatting.FeedbackAction")
.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")
Expand Down Expand Up @@ -2122,8 +2123,9 @@ private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Autom
.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(@"""$($_.FeedbackName)$($_.FeedbackName.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FeedbackName")
.AddItemScriptBlock(@"""$($_.FeedbackText)$($_.FeedbackText.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FeedbackText")
.AddItemScriptBlock(@"""$($_.FeedbackAction)$($_.FeedbackAction.Replace(""""`e"""",'`e'))$($PSStyle.Reset)""", label: "FeedbackAction")
.EndEntry()
.EndList());
}
Expand Down
24 changes: 19 additions & 5 deletions src/System.Management.Automation/FormatAndOutput/common/PSStyle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,24 +435,38 @@ public string Debug
/// <summary>
/// Gets or sets the style for rendering feedback provider names.
/// </summary>
public string FeedbackProvider
public string FeedbackName
{
get => _feedbackProvider;
set => _feedbackProvider = ValidateNoContent(value);
get => _feedbackName;
set => _feedbackName = ValidateNoContent(value);
}

private string _feedbackProvider = "\x1b[33m";
// Yellow by default.
private string _feedbackName = "\x1b[33m";

/// <summary>
/// Gets or sets the style for rendering feedback text.
/// Gets or sets the style for rendering feedback message.
/// </summary>
public string FeedbackText
{
get => _feedbackText;
set => _feedbackText = ValidateNoContent(value);
}

// BrightCyan by default.
private string _feedbackText = "\x1b[96m";

/// <summary>
/// Gets or sets the style for rendering feedback actions.
/// </summary>
public string FeedbackAction
{
get => _feedbackAction;
set => _feedbackAction = ValidateNoContent(value);
}

// BrightWhite by default.
private string _feedbackAction = "\x1b[97m";
}

/// <summary>
Expand Down
42 changes: 21 additions & 21 deletions src/System.Management.Automation/engine/Subsystem/FeedbackSubsystem/FeedbackHub.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace System.Management.Automation.Subsystem.Feedback
/// <summary>
/// The class represents a result from a feedback provider.
/// </summary>
public class FeedbackEntry
public class FeedbackResult
{
/// <summary>
/// Gets the Id of the feedback provider.
Expand All @@ -30,15 +30,15 @@ public class FeedbackEntry
public string Name { get; }

/// <summary>
/// Gets the text of the feedback.
/// Gets the feedback item.
/// </summary>
public string Text { get; }
public FeedbackItem Item { get; }

internal FeedbackEntry(Guid id, string name, string text)
internal FeedbackResult(Guid id, string name, FeedbackItem item)
{
Id = id;
Name = name;
Text = text;
Item = item;
}
}

Expand All @@ -50,15 +50,15 @@ public static class FeedbackHub
/// <summary>
/// Collect the feedback from registered feedback providers using the default timeout.
/// </summary>
public static List<FeedbackEntry>? GetFeedback(Runspace runspace)
public static List<FeedbackResult>? GetFeedback(Runspace runspace)
{
return GetFeedback(runspace, millisecondsTimeout: 300);
}

/// <summary>
/// Collect the feedback from registered feedback providers using the specified timeout.
/// </summary>
public static List<FeedbackEntry>? GetFeedback(Runspace runspace, int millisecondsTimeout)
public static List<FeedbackResult>? GetFeedback(Runspace runspace, int millisecondsTimeout)
{
Requires.Condition(millisecondsTimeout > 0, nameof(millisecondsTimeout));

Expand Down Expand Up @@ -110,9 +110,9 @@ public static class FeedbackHub
}

IFeedbackProvider? generalFeedback = null;
List<Task<FeedbackEntry?>>? tasks = null;
List<Task<FeedbackResult?>>? tasks = null;
CancellationTokenSource? cancellationSource = null;
Func<object?, FeedbackEntry?>? callBack = null;
Func<object?, FeedbackResult?>? callBack = null;

for (int i = 0; i < providers.Count; i++)
{
Expand All @@ -126,7 +126,7 @@ public static class FeedbackHub

if (tasks is null)
{
tasks = new List<Task<FeedbackEntry?>>(capacity: count);
tasks = new List<Task<FeedbackResult?>>(capacity: count);
cancellationSource = new CancellationTokenSource();
callBack = GetCallBack(lastHistory.CommandLine, lastError, cancellationSource);
}
Expand All @@ -148,7 +148,7 @@ public static class FeedbackHub
Task.Delay(millisecondsTimeout, cancellationSource!.Token));
}

List<FeedbackEntry>? resultList = null;
List<FeedbackResult>? resultList = null;
if (generalFeedback is not null)
{
bool changedDefault = false;
Expand All @@ -162,11 +162,11 @@ public static class FeedbackHub
Runspace.DefaultRunspace = localRunspace;
}

string? text = generalFeedback.GetFeedback(lastHistory.CommandLine, lastError, CancellationToken.None);
if (text is not null)
FeedbackItem? item = generalFeedback.GetFeedback(lastHistory.CommandLine, lastError, CancellationToken.None);
if (item is not null)
{
resultList ??= new List<FeedbackEntry>(count);
resultList.Add(new FeedbackEntry(generalFeedback.Id, generalFeedback.Name, text));
resultList ??= new List<FeedbackResult>(count);
resultList.Add(new FeedbackResult(generalFeedback.Id, generalFeedback.Name, item));
}
}
finally
Expand All @@ -188,14 +188,14 @@ public static class FeedbackHub
waitTask.Wait();
cancellationSource!.Cancel();

foreach (Task<FeedbackEntry?> task in tasks!)
foreach (Task<FeedbackResult?> task in tasks!)
{
if (task.IsCompletedSuccessfully)
{
FeedbackEntry? result = task.Result;
FeedbackResult? result = task.Result;
if (result is not null)
{
resultList ??= new List<FeedbackEntry>(count);
resultList ??= new List<FeedbackResult>(count);
resultList.Add(result);
}
}
Expand All @@ -212,16 +212,16 @@ public static class FeedbackHub

// A local helper function to avoid creating an instance of the generated delegate helper class
// when no feedback provider is registered.
private static Func<object?, FeedbackEntry?> GetCallBack(
private static Func<object?, FeedbackResult?> 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);
var item = provider.GetFeedback(commandLine, lastError, cancellationSource.Token);
return item is null ? null : new FeedbackResult(provider.Id, provider.Name, item);
};
}
}
Expand Down
Loading