Skip to content

Quote bare quote file paths if the path includes a double quote character #25631

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 5 commits into from
Jun 9, 2025
Merged

Quote bare quote file paths if the path includes a double quote character #25631

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e296e46
Quote bare quote file paths if the path includes a double quote chara…
MartinGC94 Jun 3, 2025
ea47802
Add tests
MartinGC94 Jun 4, 2025
2401281
Adjust comments
MartinGC94 Jun 4, 2025
ff29104
Make test clearer
MartinGC94 Jun 5, 2025
ea9dcf1
Fix test
MartinGC94 Jun 6, 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
20 changes: 15 additions & 5 deletions src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5043,14 +5043,24 @@ private static void EscapeCharIfNeeded(
break;

default:
// Handle all the different quote types
if (useSingleQuoteEscapeRules && path[index].IsSingleQuote())
if (useSingleQuoteEscapeRules)
{
_ = sb.Append('\'');
quotesAreNeeded = true;
// Bareword or singlequoted input string.
if (path[index].IsSingleQuote())
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Jun 5, 2025

Choose a reason for hiding this comment

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

It seems the method would be more readable if we ridded of useSingleQuoteEscapeRules and replace it with stringType is StringConstantType.SingleQuoted or StringConstantType.BareWord.

Copy link
Copy Markdown
Contributor Author

@MartinGC94 MartinGC94 Jun 5, 2025

Choose a reason for hiding this comment

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

That check will change when my PR to keep variables when tab completing paths is merged. If someone is tab completing a bareword string with a variable it needs to use double quote rules.

{
// SingleQuotes are escaped with more single quotes. quotesAreNeeded is set so bareword strings can quoted.
_ = sb.Append('\'');
quotesAreNeeded = true;
}
else if (!quotesAreNeeded && stringType == StringConstantType.BareWord && path[index].IsDoubleQuote())
{
// Bareword string with double quote inside. Make sure to quote it so we don't need to escape it.
quotesAreNeeded = true;
}
}
else if (!useSingleQuoteEscapeRules && path[index].IsDoubleQuote())
else if (path[index].IsDoubleQuote())
{
// Double quoted or bareword with variables input string. Need to escape double quotes.
_ = sb.Append('`');
quotesAreNeeded = true;
}
Expand Down
34 changes: 34 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 @@ -2257,6 +2257,40 @@ param ($Param1)

Remove-Item -LiteralPath $LiteralPath
}

It "Should add single quotes if there are double quotes in bare word file path" {
$BadQuote = [char]8220
$TestFile1 = Join-Path -Path $TestDrive -ChildPath "Test1${BadQuote}File"
Comment thread
MartinGC94 marked this conversation as resolved.
$null = New-Item -Path $TestFile1 -Force
$res = TabExpansion2 -inputScript "Get-ChildItem -Path $TestDrive\"
($res.CompletionMatches | Where-Object ListItemText -Like "Test1?File").CompletionText | Should -Be "'$TestFile1'"
Remove-Item -LiteralPath $TestFile1 -Force
}

It "Should escape double quote if the input string uses double quotes" {
$BadQuote = [char]8220
$TestFile1 = Join-Path -Path $TestDrive -ChildPath "Test1${BadQuote}File"
$null = New-Item -Path $TestFile1 -Force
$res = TabExpansion2 -inputScript "Get-ChildItem -Path `"$TestDrive\"
$Expected = "`"$($TestFile1.Insert($TestFile1.LastIndexOf($BadQuote), '`'))`""
($res.CompletionMatches | Where-Object ListItemText -Like "Test1?File").CompletionText | Should -Be $Expected
Remove-Item -LiteralPath $TestFile1 -Force
}

It "Should escape single quotes in file paths" {
$BadQuote = "'"
$TestFile1 = Join-Path -Path $TestDrive -ChildPath "Test1${BadQuote}File"
$null = New-Item -Path $TestFile1 -Force
$Expected = "'$($TestFile1.Insert($TestFile1.LastIndexOf($BadQuote), "'"))'"

$res = TabExpansion2 -inputScript "Get-ChildItem -Path '$TestDrive\"
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Jun 5, 2025

Choose a reason for hiding this comment

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

If it is intentional to have single quote it is not obvious so please add a comment.

($res.CompletionMatches | Where-Object ListItemText -Like "Test1?File").CompletionText | Should -Be $Expected

$res = TabExpansion2 -inputScript "Get-ChildItem -Path $TestDrive\"
($res.CompletionMatches | Where-Object ListItemText -Like "Test1?File").CompletionText | Should -Be $Expected

Remove-Item -LiteralPath $TestFile1 -Force
}
}

It 'Should correct slashes in UNC path completion' -Skip:(!$IsWindows) {
Expand Down
Loading