Fix WildcardPattern.Escape to escape lone backticks correctly#25211
Fix WildcardPattern.Escape to escape lone backticks correctly#25211iSazonov merged 8 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom ArmaanMcleod:fix-wildcard-escape-v2ArmaanMcleod/PowerShell:fix-wildcard-escape-v2Copy head branch name to clipboard
WildcardPattern.Escape to escape lone backticks correctly#25211Conversation
WildcardPattern.Escape for backticksWildcardPattern.Escape to escape lone backticks correctly
| @{ lhs = 'abc`def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc`def'); result = $true } | ||
| @{ lhs = 'abc`def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc``def'); result = $false } | ||
| @{ lhs = 'abc``def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc``def'); result = $true } | ||
| @{ lhs = 'abc``def'; operator = '-like'; rhs = [WildcardPattern]::Escape('abc````def'); result = $false } |
There was a problem hiding this comment.
There is "Win7 backcompatibility requires treating '`' pattern as '' pattern" comment in regex.cs file. Please add a test for the scenario.
There was a problem hiding this comment.
@iSazonov Do we need to worry about this? I am not sure why we are still trying to support Win 7 compatibility and why such code even exists 😄. Can you elaborate on what scenario this covers?
There was a problem hiding this comment.
Given also the code changes here are in Escape() I am not sure what I am testing. The Win7 compatibility is more with ensuring WildcardPattern in Parse() method treats lone backtick as empty string.
|
@IISResetMe Could you please review? |
|
Changes look good to me! |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
📣 Hey @ArmaanMcleod, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
|
Amazing. Thanks @iSazonov & @IISResetMe for the reviews and discussion on this one. It took a couple tries but I think we got there in the end 😄. Learnt a lot more how PowerShell does escaping internally too. |
|
We are waiting feedback from community now. If there are no critical messages, then this will be included in the new version. |
PR Summary
Fix
WildcardPattern.Escapeto escape lone backticks correctly.I have added the
-likespecific tests in Pester and theEscape()method conversion tests in XUnit.Given backtick is included in
SpecialCharswhich also has wildcard chars, it seems more straightforward to just doSpecialChars.Contains(ch)instead ofIsWildcardChar(ch) || ch == escapeCharin theEscape()method.PR Context
Fixes #16306
cc @IISResetMe
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.- [ ] Issue filed:
(which runs in a different PS Host).