Skip to content

Conversation

@adityapatwardhan
Copy link
Member

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

  • Required tooling change
  • Optional tooling change (include reasoning)

Customer Impact

  • Customer reported
  • Found internally

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.

  • Yes
  • No

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.

  • High
  • Medium
  • Low

Feature addition with backwards compatibility, isolated to Get-Clipboard cmdlet with comprehensive test coverage. No breaking changes or core engine modifications.

Copilot AI review requested due to automatic review settings December 4, 2025 18:14
@adityapatwardhan adityapatwardhan added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Dec 4, 2025
@adityapatwardhan adityapatwardhan enabled auto-merge (squash) December 4, 2025 18:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 Delimiter parameter to Get-Clipboard cmdlet with default value of [Environment.NewLine]
  • Implements DelimiterCompleter class 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.

Comment on lines +39 to +44
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}
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

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"
}

Copilot uses AI. Check for mistakes.
/// Gets or sets the delimiters to use when splitting the clipboard content.
/// </summary>
[Parameter]
[ArgumentCompleter(typeof(DelimiterCompleter))]
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
[ArgumentCompleter(typeof(DelimiterCompleter))]
[ArgumentCompleter(typeof(DelimiterCompleter))]
[ValidateNotNullOrEmpty]

Copilot uses AI. Check for mistakes.
Comment on lines +90 to +114
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)");
}
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[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.

Copilot uses AI. Check for mistakes.
@adityapatwardhan adityapatwardhan merged commit 2d213e2 into PowerShell:release/v7.6 Dec 4, 2025
37 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants