Skip to content

Refactor Tooltip/ListItemText mapping to use CompletionDisplayInfoMapper delegate #25395

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
iSazonov merged 9 commits into from
Apr 21, 2025
Merged

Refactor Tooltip/ListItemText mapping to use CompletionDisplayInfoMapper delegate #25395

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a405893
Use CompletionDisplayInfoMapper delegate for display info mapping
ArmaanMcleod Apr 21, 2025
b6a0e60
Update src/Microsoft.PowerShell.Commands.Management/commands/manageme…
ArmaanMcleod Apr 21, 2025
96170ce
Add tests for tooltip and listitemtext
ArmaanMcleod Apr 21, 2025
fee92b4
Make NewLineText const
ArmaanMcleod Apr 21, 2025
f0c21a2
Remove class and use tuple
ArmaanMcleod Apr 21, 2025
1da96c0
Use named tuples and format on multiple lines for easier readability
ArmaanMcleod Apr 21, 2025
0f12e06
Add check for tooltip not null or empty
ArmaanMcleod Apr 21, 2025
89285c5
Update src/System.Management.Automation/engine/CommandCompletion/Comp…
ArmaanMcleod Apr 21, 2025
d1e809c
SA1009 Codefactor: Closing parenthesis should not be preceded by a sp…
ArmaanMcleod Apr 21, 2025
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
47 changes: 34 additions & 13 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/NewPropertyCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,39 @@ protected override void ProcessRecord()
/// </summary>
public class PropertyTypeArgumentCompleter : IArgumentCompleter
{
private static readonly string[] s_RegistryPropertyTypes = new string[]
private static readonly CompletionHelpers.CompletionDisplayInfoMapper RegistryPropertyTypeDisplayInfoMapper = registryPropertyType => registryPropertyType switch
{
"String" => (
ToolTip: TabCompletionStrings.RegistryStringToolTip,
ListItemText: "String"
),
"ExpandString" => (
ToolTip: TabCompletionStrings.RegistryExpandStringToolTip,
ListItemText: "ExpandString"
),
"Binary" => (
ToolTip: TabCompletionStrings.RegistryBinaryToolTip,
ListItemText: "Binary"
),
"DWord" => (
ToolTip: TabCompletionStrings.RegistryDWordToolTip,
ListItemText: "DWord"
),
"MultiString" => (
ToolTip: TabCompletionStrings.RegistryMultiStringToolTip,
ListItemText: "MultiString"
),
"QWord" => (
ToolTip: TabCompletionStrings.RegistryQWordToolTip,
ListItemText: "QWord"
),
_ => (
ToolTip: TabCompletionStrings.RegistryUnknownToolTip,
ListItemText: "Unknown"
)
};

private static readonly IReadOnlyList<string> s_RegistryPropertyTypes = new List<string>(capacity: 7)
{
"String",
"ExpandString",
Expand All @@ -200,17 +232,6 @@ public class PropertyTypeArgumentCompleter : IArgumentCompleter
"Unknown"
};

private static string GetRegistryPropertyTypeToolTip(string propertyTypeName) => propertyTypeName switch
{
"String" => TabCompletionStrings.RegistryStringToolTip,
"ExpandString" => TabCompletionStrings.RegistryExpandStringToolTip,
"Binary" => TabCompletionStrings.RegistryBinaryToolTip,
"DWord" => TabCompletionStrings.RegistryDWordToolTip,
"MultiString" => TabCompletionStrings.RegistryMultiStringToolTip,
"QWord" => TabCompletionStrings.RegistryQWordToolTip,
_ => TabCompletionStrings.RegistryUnknownToolTip
};

/// <summary>
/// Returns completion results for PropertyType parameter.
/// </summary>
Expand All @@ -230,7 +251,7 @@ public IEnumerable<CompletionResult> CompleteArgument(
? CompletionHelpers.GetMatchingResults(
wordToComplete,
possibleCompletionValues: s_RegistryPropertyTypes,
toolTipMapping: GetRegistryPropertyTypeToolTip,
displayInfoMapper: RegistryPropertyTypeDisplayInfoMapper,
resultType: CompletionResultType.ParameterValue)
: [];

Expand Down
67 changes: 45 additions & 22 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/Join-String.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,35 +165,59 @@ protected override void EndProcessing()
/// </summary>
public sealed class SeparatorArgumentCompleter : IArgumentCompleter
{
private static readonly string NewLineText =
private const string NewLineText =
#if UNIX
"`n";
#else
"`r`n";
#endif

private static readonly Dictionary<string, (string Tooltip, string ListItemText)> s_separatorMappings = new(capacity: 7)
private static readonly CompletionHelpers.CompletionDisplayInfoMapper SeparatorDisplayInfoMapper = separator => separator switch
{
{ ",", (TabCompletionStrings.SeparatorCommaToolTip, "Comma") },
{ ", ", (TabCompletionStrings.SeparatorCommaSpaceToolTip, "Comma-Space") },
{ ";", (TabCompletionStrings.SeparatorSemiColonToolTip, "Semi-Colon") },
{ "; ", (TabCompletionStrings.SeparatorSemiColonSpaceToolTip, "Semi-Colon-Space") },
{ NewLineText, (StringUtil.Format(TabCompletionStrings.SeparatorNewlineToolTip, NewLineText), "Newline") },
{ "-", (TabCompletionStrings.SeparatorDashToolTip, "Dash") },
{ " ", (TabCompletionStrings.SeparatorSpaceToolTip, "Space") },
"," => (
ToolTip: TabCompletionStrings.SeparatorCommaToolTip,
ListItemText: "Comma"
),
", " => (
ToolTip: TabCompletionStrings.SeparatorCommaSpaceToolTip,
ListItemText: "Comma-Space"
),
";" => (
ToolTip: TabCompletionStrings.SeparatorSemiColonToolTip,
ListItemText: "Semi-Colon"
),
"; " => (
ToolTip: TabCompletionStrings.SeparatorSemiColonSpaceToolTip,
ListItemText: "Semi-Colon-Space"
),
"-" => (
ToolTip: TabCompletionStrings.SeparatorDashToolTip,
ListItemText: "Dash"
),
" " => (
ToolTip: TabCompletionStrings.SeparatorSpaceToolTip,
ListItemText: "Space"
),
NewLineText => (
ToolTip: StringUtil.Format(TabCompletionStrings.SeparatorNewlineToolTip, NewLineText),
ListItemText: "Newline"
),
_ => (
ToolTip: separator,
ListItemText: separator
)
};

private static readonly IEnumerable<string> s_separatorValues = s_separatorMappings.Keys;

private static string GetSeparatorToolTip(string separator)
=> s_separatorMappings.TryGetValue(separator, out var mapping)
? mapping.Tooltip
: separator;

private static string GetSeparatorListItemText(string separator)
=> s_separatorMappings.TryGetValue(separator, out var mapping)
? mapping.ListItemText
: separator;
private static readonly IReadOnlyList<string> s_separatorValues = new List<string>(capacity: 7)
{
",",
", ",
";",
"; ",
NewLineText,
"-",
" ",
};

/// <summary>
/// Returns completion results for Separator parameter.
Expand All @@ -213,8 +237,7 @@ public IEnumerable<CompletionResult> CompleteArgument(
=> CompletionHelpers.GetMatchingResults(
wordToComplete,
possibleCompletionValues: s_separatorValues,
listItemTextMapping: GetSeparatorListItemText,
toolTipMapping: GetSeparatorToolTip,
displayInfoMapper: SeparatorDisplayInfoMapper,
resultType: CompletionResultType.ParameterValue);
}

Expand Down
29 changes: 23 additions & 6 deletions src/System.Management.Automation/engine/CommandCompletion/CompletionHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,18 @@ internal static class CompletionHelpers
/// </summary>
/// <param name="wordToComplete">The word to complete.</param>
/// <param name="possibleCompletionValues">The possible completion values to iterate.</param>
/// <param name="toolTipMapping">The optional tool tip mapping delegate.</param>
/// <param name="listItemTextMapping">The optional list item text mapping delegate.</param>
/// <param name="displayInfoMapper">The optional completion display info mapper delegate for tool tip and list item text.</param>
/// <param name="resultType">The optional completion result type. Default is Text.</param>
/// <param name="matchStrategy">The optional match strategy delegate.</param>
/// <returns>List of matching completion results.</returns>
internal static IEnumerable<CompletionResult> GetMatchingResults(
string wordToComplete,
IEnumerable<string> possibleCompletionValues,
Func<string, string> toolTipMapping = null,
Func<string, string> listItemTextMapping = null,
CompletionDisplayInfoMapper displayInfoMapper = null,
CompletionResultType resultType = CompletionResultType.Text,
MatchStrategy matchStrategy = null)
{
displayInfoMapper ??= DefaultDisplayInfoMapper;
matchStrategy ??= DefaultMatch;

string quote = HandleDoubleAndSingleQuote(ref wordToComplete);
Expand All @@ -49,14 +48,32 @@ internal static IEnumerable<CompletionResult> GetMatchingResults(
if (matchStrategy(value, wordToComplete))
{
string completionText = QuoteCompletionText(value, quote);
string toolTip = toolTipMapping?.Invoke(value) ?? value;
string listItemText = listItemTextMapping?.Invoke(value) ?? value;

(string ToolTip, string ListItemText) displayInfo = displayInfoMapper(value);
string toolTip = displayInfo.ToolTip;
string listItemText = displayInfo.ListItemText;
Comment thread
ArmaanMcleod marked this conversation as resolved.
Outdated

yield return new CompletionResult(completionText, listItemText, resultType, toolTip);
}
}
}

/// <summary>
/// Provides the display information for a completion result.
/// This delegate is used to map a string value to its corresponding display information.
/// </summary>
/// <param name="value">The input value to be mapped</param>
/// <returns>Completion display info containing tool tip and list item text.</returns>
internal delegate (string ToolTip, string ListItemText) CompletionDisplayInfoMapper(string value);

/// <summary>
/// Provides the default display information for a completion result.
/// Defaults to using the input value for both the tool tip and list item text.
/// </summary>
/// <returns>Completion display info containing tool tip and list item text.</returns>
internal static readonly CompletionDisplayInfoMapper DefaultDisplayInfoMapper = value
=> (value, value);

/// <summary>
/// Normalizes the input string to an expandable string format for PowerShell.
/// </summary>
Expand Down
31 changes: 31 additions & 0 deletions test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,13 @@ param([ValidatePattern(
$res = TabExpansion2 -inputScript $TextInput -cursorColumn $TextInput.Length
$completionText = $res.CompletionMatches.CompletionText
$completionText -join ' ' | Should -BeExactly $ExpectedPropertyTypes

foreach ($match in $res.CompletionMatches) {
$completionText = $match.CompletionText.Replace("""", "").Replace("'", "")
$listItemText = $match.ListItemText
$completionText | Should -BeExactly $listItemText
$match.ToolTip | Should -Not -BeNullOrEmpty
}
Comment thread
ArmaanMcleod marked this conversation as resolved.
}

It "Test fallback to provider of current location if no path specified" -Skip:(!$IsWindows) {
Expand Down Expand Up @@ -1554,6 +1561,14 @@ param([ValidatePattern(
$res = TabExpansion2 -inputScript $TextInput -cursorColumn $TextInput.Length
$completionText = $res.CompletionMatches.CompletionText
$completionText -join ' ' | Should -BeExactly $Expected

foreach ($match in $res.CompletionMatches) {
$toolTip = $match.ToolTip.Replace("""", "").Replace("'", "")
$completionText = $match.CompletionText.Replace("""", "").Replace("'", "")
$listItemText = $match.ListItemText
$toolTip.StartsWith($completionText) | Should -BeTrue
$toolTip.EndsWith($listItemText) | Should -BeTrue
}
}

It "Should complete for '<TextInput>'" -Skip:(!$IsWindows) -TestCases @(
Expand All @@ -1574,6 +1589,14 @@ param([ValidatePattern(
$res = TabExpansion2 -inputScript $TextInput -cursorColumn $TextInput.Length
$completionText = $res.CompletionMatches.CompletionText
$completionText -join ' ' | Should -BeExactly $Expected

foreach ($match in $res.CompletionMatches) {
$toolTip = $match.ToolTip.Replace("""", "").Replace("'", "")
$completionText = $match.CompletionText.Replace("""", "").Replace("'", "")
$listItemText = $match.ListItemText
$toolTip.StartsWith($completionText) | Should -BeTrue
$toolTip.EndsWith($listItemText) | Should -BeTrue
}
}

It "Should complete for '<TextInput>'" -Skip:($IsWindows) -TestCases @(
Expand All @@ -1590,6 +1613,14 @@ param([ValidatePattern(
$res = TabExpansion2 -inputScript $TextInput -cursorColumn $TextInput.Length
$completionText = $res.CompletionMatches.CompletionText
$completionText -join ' ' | Should -BeExactly $Expected

foreach ($match in $res.CompletionMatches) {
$toolTip = $match.ToolTip.Replace("""", "").Replace("'", "")
$completionText = $match.CompletionText.Replace("""", "").Replace("'", "")
$listItemText = $match.ListItemText
$toolTip.StartsWith($completionText) | Should -BeTrue
$toolTip.EndsWith($listItemText) | Should -BeTrue
}
}
}

Expand Down
Loading