-
Notifications
You must be signed in to change notification settings - Fork 521
Reduce scope of regex in PowerShellExeFinder #5228
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.
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
Reduce scope of regex in PowerShellExeFinder #5228
Conversation
@Copilot
Copilot
AI
left a comment
There was a problem hiding this 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 updates the integer checking regex in the PowerShellExeFinder to enforce a stricter validation by matching exactly a single digit rather than one or more digits.
- Change the IntRegex from matching one or more digits (/^\d+$/) to exactly one digit (/^\d$/).
- This adjustment aligns the implementation with the implied expectation from related comments regarding folder naming.
Comments suppressed due to low confidence (1)
src/platform.ts:81
- Consider updating the associated comment to clearly state that the regex now only supports single digit folder names, ensuring that the documentation is consistent with the updated regex behavior.
private static IntRegex = /^\d$/;
Hi, is there a known bug that this code change addresses?
Nope! Not explicitly anyways. The tests imply that it should be looking for a single digit integer value, but it would really find any integer value.
I was just trying to match the test expectations for clarity purposes.
Al righty, without a reproducible bug I'm going to close this. All code changes introduce risk, so without something identifiable as "fixed" it's not worth introducing such changes. Thanks though!
PR Summary
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready