Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2567,9 +2567,8 @@ private static ScriptBlock GetCustomArgumentCompleter(
}
}

var registeredCompleters = optionKey.Equals("NativeArgumentCompleters", StringComparison.OrdinalIgnoreCase)
? context.NativeArgumentCompleters
: context.CustomArgumentCompleters;
bool isNative = optionKey.Equals("NativeArgumentCompleters", StringComparison.OrdinalIgnoreCase);
var registeredCompleters = isNative ? context.NativeArgumentCompleters : context.CustomArgumentCompleters;

if (registeredCompleters != null)
{
Expand All @@ -2580,6 +2579,13 @@ private static ScriptBlock GetCustomArgumentCompleter(
return scriptBlock;
}
}

// For a native command, if a fallback completer is registered, then return it.
// For example, the 'Microsoft.PowerShell.UnixTabCompletion' module.
if (isNative && registeredCompleters.TryGetValue(RegisterArgumentCompleterCommand.FallbackCompleterKey, out scriptBlock))
{
return scriptBlock;
}
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,66 +172,110 @@ public abstract class ArgumentCompleterFactoryAttribute : ArgumentCompleterAttri
[Cmdlet(VerbsLifecycle.Register, "ArgumentCompleter", HelpUri = "https://go.microsoft.com/fwlink/?LinkId=528576")]
public class RegisterArgumentCompleterCommand : PSCmdlet
{
private const string PowerShellSetName = "PowerShellSet";
private const string NativeCommandSetName = "NativeCommandSet";
private const string NativeFallbackSetName = "NativeFallbackSet";

// Use a key that is unlikely to be a file name or path to indicate the fallback completer for native commands.
internal const string FallbackCompleterKey = "___ps::<native_fallback_key>@@___";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary cannot take $null as the key, so I have to create a special key to indicate the fallback completer.


/// <summary>
/// Gets or sets the command names for which the argument completer is registered.
/// </summary>
[Parameter(ParameterSetName = "NativeSet", Mandatory = true)]
[Parameter(ParameterSetName = "PowerShellSet")]
[Parameter(ParameterSetName = NativeCommandSetName, Mandatory = true)]
[Parameter(ParameterSetName = PowerShellSetName)]
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public string[] CommandName { get; set; }

/// <summary>
/// Gets or sets the name of the parameter for which the argument completer is registered.
/// </summary>
[Parameter(ParameterSetName = "PowerShellSet", Mandatory = true)]
[Parameter(ParameterSetName = PowerShellSetName, Mandatory = true)]
public string ParameterName { get; set; }

/// <summary>
/// Gets or sets the script block that will be executed to provide argument completions.
/// </summary>
[Parameter(Mandatory = true)]
[AllowNull()]
public ScriptBlock ScriptBlock { get; set; }

/// <summary>
/// Indicates the argument completer is for native commands.
/// </summary>
[Parameter(ParameterSetName = "NativeSet")]
[Parameter(ParameterSetName = NativeCommandSetName)]
public SwitchParameter Native { get; set; }

/// <summary>
/// Indicates the argument completer is a fallback for any native commands that don't have a completer registered.
/// </summary>
[Parameter(ParameterSetName = NativeFallbackSetName)]
public SwitchParameter NativeFallback { get; set; }

/// <summary>
/// </summary>
protected override void EndProcessing()
{
Dictionary<string, ScriptBlock> completerDictionary;
if (ParameterName != null)

if (ParameterSetName is NativeFallbackSetName)
{
completerDictionary = Context.CustomArgumentCompleters ??
(Context.CustomArgumentCompleters = new Dictionary<string, ScriptBlock>(StringComparer.OrdinalIgnoreCase));
completerDictionary = Context.NativeArgumentCompleters ??= new(StringComparer.OrdinalIgnoreCase);

SetKeyValue(completerDictionary, FallbackCompleterKey, ScriptBlock);
}
else
else if (ParameterSetName is NativeCommandSetName)
{
completerDictionary = Context.NativeArgumentCompleters ??
(Context.NativeArgumentCompleters = new Dictionary<string, ScriptBlock>(StringComparer.OrdinalIgnoreCase));
}
completerDictionary = Context.NativeArgumentCompleters ??= new(StringComparer.OrdinalIgnoreCase);

if (CommandName == null || CommandName.Length == 0)
foreach (string command in CommandName)
{
var key = command?.Trim();
if (string.IsNullOrEmpty(key))
{
continue;
}

SetKeyValue(completerDictionary, key, ScriptBlock);
}
}
else if (ParameterSetName is PowerShellSetName)
{
CommandName = new[] { string.Empty };
completerDictionary = Context.CustomArgumentCompleters ??= new(StringComparer.OrdinalIgnoreCase);

string paramName = ParameterName.Trim();
if (paramName.Length is 0)
{
return;
}

if (CommandName is null || CommandName.Length is 0)
{
SetKeyValue(completerDictionary, paramName, ScriptBlock);
return;
}

foreach (string command in CommandName)
{
var key = command?.Trim();
key = string.IsNullOrEmpty(key)
? paramName
: $"{key}:{paramName}";

SetKeyValue(completerDictionary, key, ScriptBlock);
}
}

for (int i = 0; i < CommandName.Length; i++)
static void SetKeyValue(Dictionary<string, ScriptBlock> table, string key, ScriptBlock value)
{
var key = CommandName[i];
if (!string.IsNullOrWhiteSpace(ParameterName))
if (value is null)
{
if (!string.IsNullOrWhiteSpace(key))
{
key = key + ":" + ParameterName;
}
else
{
key = ParameterName;
}
table.Remove(key);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that I'm suggesting a larger change here, but the pattern is typically Get-* | Unregister-*. Treating null as remove will potentially hide some design time errors like passing a misspelled variable name.

I definitely understand the need but I'd suggest tackling that separately

Copy link
Member Author

@daxian-dbw daxian-dbw May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree there should be Get-ArgumentCompleter and Unregister-ArgumentCompleter cmdlets for better discoverability and manageability! I will open an issue to track.

For Register-ArgumentCompleter, today its behavior when specifying -ScriptBlock $null is to blindly set null to completerDictionary[key]. It essentially disables the previous setting for key without clearing that entry. So, change in this PR only just clear that key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #25800 (Consider adding Get-ArgumentCompleter and Unregister-ArgumentCompleter for custom completer management)

}
else
{
table[key] = value;
}

completerDictionary[key] = ScriptBlock;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1067,9 +1067,9 @@ internal override void SetHistoryString(string historyString)
/// ExecutionContext, if it available in TLS
/// Null, if ExecutionContext is not available in TLS
/// </returns>
internal static System.Management.Automation.ExecutionContext GetExecutionContextFromTLS()
internal static ExecutionContext GetExecutionContextFromTLS()
{
System.Management.Automation.Runspaces.Runspace runspace = Runspace.DefaultRunspace;
Runspace runspace = Runspace.DefaultRunspace;
if (runspace == null)
{
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ internal static ScriptBlock Create(ExecutionContext context, string script)
/// </summary>
/// <param name="script">The string to compile.</param>
public static ScriptBlock Create(string script) => Create(
parser: new Language.Parser(),
parser: new Parser(),
fileName: null,
fileContents: script);

Expand Down
52 changes: 51 additions & 1 deletion test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1772,8 +1772,12 @@ param([ValidatePattern(

Context NativeCommand {
BeforeAll {
$nativeCommand = (Get-Command -CommandType Application -TotalCount 1).Name
## Find a native command that is not 'pwsh'. We will use 'pwsh' for fallback completer tests later.
$nativeCommand = Get-Command -CommandType Application -TotalCount 2 |
Where-Object Name -NotLike pwsh* |
Select-Object -First 1
}

It 'Completes native commands with -' {
Register-ArgumentCompleter -Native -CommandName $nativeCommand -ScriptBlock {
param($wordToComplete, $ast, $cursorColumn)
Expand Down Expand Up @@ -1837,6 +1841,52 @@ param([ValidatePattern(
$res.CompletionMatches | Should -HaveCount 1
$res.CompletionMatches.CompletionText | Should -BeExactly "-option"
}

It 'Covers an arbitrary unbound native command with -t' {
## Register a completer for $nativeCommand.
Register-ArgumentCompleter -Native -CommandName $nativeCommand -ScriptBlock {
param($wordToComplete, $ast, $cursorColumn)
if ($wordToComplete -eq '-t') {
return "-terminal"
}
}

## Register a fallback native command completer.
Register-ArgumentCompleter -NativeFallback -ScriptBlock {
param($wordToComplete, $ast, $cursorColumn)
if ($wordToComplete -eq '-t') {
return "-testing"
}
}

## The specific completer will be used if it exists.
$line = "$nativeCommand -t"
$res = TabExpansion2 -inputScript $line -cursorColumn $line.Length
$res.CompletionMatches | Should -HaveCount 1
$res.CompletionMatches.CompletionText | Should -BeExactly "-terminal"

## Otherwise, the fallback completer will kick in.
$line = "pwsh -t"
$res = TabExpansion2 -inputScript $line -cursorColumn $line.Length
$res.CompletionMatches | Should -HaveCount 1
$res.CompletionMatches.CompletionText | Should -BeExactly "-testing"

## Remove the completer for $nativeCommand.
Register-ArgumentCompleter -Native -CommandName $nativeCommand -ScriptBlock $null

## The fallback completer will be used for $nativeCommand.
$line = "$nativeCommand -t"
$res = TabExpansion2 -inputScript $line -cursorColumn $line.Length
$res.CompletionMatches | Should -HaveCount 1
$res.CompletionMatches.CompletionText | Should -BeExactly "-testing"

## Remove the fallback completer for $nativeCommand.
Register-ArgumentCompleter -NativeFallback -ScriptBlock $null

## The fallback completer will be used for $nativeCommand.
$res = TabExpansion2 -inputScript $line -cursorColumn $line.Length
$res.CompletionMatches | Should -HaveCount 0
}
}

It 'Should complete "Export-Counter -FileFormat" with available output formats' -Pending {
Expand Down
Loading