[release/v7.6] Add Delimiter parameter to Get-Clipboard#26572
Conversation
There was a problem hiding this comment.
Pull request overview
This PR backports the addition of a Delimiter parameter to the Get-Clipboard cmdlet from the main branch to release/v7.6. The parameter allows users to customize how clipboard content is split into individual items, enhancing cross-platform line ending handling.
Key Changes
- Adds
Delimiterparameter toGet-Clipboardcmdlet with default value of[Environment.NewLine] - Implements
DelimiterCompleterclass providing tab completion for common delimiters (CRLF, LF) - Maintains backward compatibility by defaulting to current behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Microsoft.PowerShell.Commands.Management/commands/management/GetClipboardCommand.cs | Adds Delimiter parameter with collection expression initializer and custom DelimiterCompleter for tab completion |
| test/powershell/Modules/Microsoft.PowerShell.Management/Clipboard.Tests.ps1 | Adds test validating delimiter functionality with mixed line endings (\r\n and \n) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| It 'Get-Clipboard -Delimiter should return items based on the delimiter' { | ||
| Set-Clipboard -Value "Line1`r`nLine2`nLine3" | ||
| $result = Get-Clipboard -Delimiter "`r`n", "`n" | ||
| $result.Count | Should -Be 3 | ||
| $result | ForEach-Object -Process {$_.Length | Should -Be "LineX".Length} | ||
| } |
There was a problem hiding this comment.
The test validates that the new -Delimiter parameter works correctly with mixed line endings. However, there's no test coverage for the interaction between -Raw and -Delimiter parameters. When both are specified, the -Raw parameter takes precedence (ignoring -Delimiter), but this behavior should be tested to document and validate this design decision.
Consider adding a test case like:
It 'Get-Clipboard -Raw ignores -Delimiter parameter' {
Set-Clipboard -Value "Line1`r`nLine2"
$result = Get-Clipboard -Raw -Delimiter "`n"
$result.Count | Should -Be 1
$result | Should -BeExactly "Line1`r`nLine2"
}| /// Gets or sets the delimiters to use when splitting the clipboard content. | ||
| /// </summary> | ||
| [Parameter] | ||
| [ArgumentCompleter(typeof(DelimiterCompleter))] |
There was a problem hiding this comment.
The Delimiter parameter doesn't have validation to prevent empty or null values. If a user specifies -Delimiter @() or -Delimiter @($null), the String.Split method at line 80 would throw an exception. Consider adding [ValidateNotNullOrEmpty] attribute to the parameter to provide a clearer error message:
[Parameter]
[ArgumentCompleter(typeof(DelimiterCompleter))]
[ValidateNotNullOrEmpty]
public string[] Delimiter { get; set; } = [Environment.NewLine];This would validate that the array itself is not null or empty, and PowerShell's built-in validation already prevents individual null elements in string arrays.
| [ArgumentCompleter(typeof(DelimiterCompleter))] | |
| [ArgumentCompleter(typeof(DelimiterCompleter))] | |
| [ValidateNotNullOrEmpty] |
| public sealed class DelimiterCompleter : IArgumentCompleter | ||
| { | ||
| /// <summary> | ||
| /// Provides argument completion for the Delimiter parameter. | ||
| /// </summary> | ||
| /// <param name="commandName">The name of the command that is being completed.</param> | ||
| /// <param name="parameterName">The name of the parameter that is being completed.</param> | ||
| /// <param name="wordToComplete">The input text to filter the results by.</param> | ||
| /// <param name="commandAst">The ast of the command that triggered the completion.</param> | ||
| /// <param name="fakeBoundParameters">The parameters bound to the command.</param> | ||
| /// <returns>Completion results.</returns> | ||
| public IEnumerable<CompletionResult> CompleteArgument(string commandName, string parameterName, string wordToComplete, CommandAst commandAst, IDictionary fakeBoundParameters) | ||
| { | ||
| wordToComplete ??= string.Empty; | ||
| var pattern = new WildcardPattern(wordToComplete + '*', WildcardOptions.IgnoreCase); | ||
| if (pattern.IsMatch("CRLF") || pattern.IsMatch("Windows")) | ||
| { | ||
| yield return new CompletionResult("\"`r`n\"", "CRLF", CompletionResultType.ParameterValue, "Windows (CRLF)"); | ||
| } | ||
|
|
||
| if (pattern.IsMatch("LF") || pattern.IsMatch("Unix") || pattern.IsMatch("Linux")) | ||
| { | ||
| yield return new CompletionResult("\"`n\"", "LF", CompletionResultType.ParameterValue, "UNIX (LF)"); | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The DelimiterCompleter implementation uses manual WildcardPattern matching and yield return for completion results. This codebase has a CompletionHelpers.GetMatchingResults helper method (used in SeparatorArgumentCompleter in Join-String.cs) that provides consistent handling of quoting, escaping, and pattern matching for completions. Consider using it for consistency:
private static readonly IReadOnlyList<string> s_delimiterValues = new List<string>(capacity: 2)
{
"\"`r`n\"",
"\"`n\""
};
private static readonly Dictionary<string, (string listItemText, string toolTip)> s_delimiterDisplayInfo = new()
{
{ "\"`r`n\"", ("CRLF", "Windows (CRLF)") },
{ "\"`n\"", ("LF", "UNIX (LF)") }
};
public IEnumerable<CompletionResult> CompleteArgument(...)
{
wordToComplete ??= string.Empty;
return CompletionHelpers.GetMatchingResults(
wordToComplete,
possibleCompletionValues: s_delimiterValues,
displayInfoMapper: value => s_delimiterDisplayInfo.TryGetValue(value, out var info) ? info : (value, value),
resultType: CompletionResultType.ParameterValue);
}However, the current implementation has custom logic to match against user-friendly terms like "CRLF", "Windows", "Unix", "Linux", "LF" which wouldn't work with the helper as-is. The current approach is acceptable, but the filtering logic could be simplified to be more robust.
2d213e2
into
PowerShell:release/v7.6
Backport of #26134 to release/v7.6
Triggered by @adityapatwardhan on behalf of @MartinGC94
Original CL Label: CL-General
/cc @PowerShell/powershell-maintainers
Impact
REQUIRED: Choose either Tooling Impact or Customer Impact (or both). At least one checkbox must be selected.
Tooling Impact
Customer Impact
Adds new Delimiter parameter to Get-Clipboard cmdlet allowing users to customize delimiters for splitting clipboard content. This enhances functionality for handling different line endings across platforms.
Regression
REQUIRED: Check exactly one box.
This is not a regression.
Testing
New tests added in Clipboard.Tests.ps1 to verify the new Delimiter parameter functionality. The backport preserves all existing functionality while adding the new parameter capability.
Risk
REQUIRED: Check exactly one box.
Feature addition with backwards compatibility, isolated to Get-Clipboard cmdlet with comprehensive test coverage. No breaking changes or core engine modifications.