-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
Make hexidecimal named scripts execute without requiring .\ prefix #18349
Conversation
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.
Fix seems good to me, thanks for fixing this up!! 💖 😊
Test needs a quick once over but other than that this is all good.
The change is cuausing some unexpected behavior - https://dev.azure.com/powershell/PowerShell/_build/results?buildId=111934&view=ms.vss-test-web.build-test-results-tab&runId=1961790&resultId=108814&paneView=debug I'm not sure how this can be related at all to the changes I made. |
I hope @vexx32 and @SeeminglyScience could help. |
🤔 Not seeing anything in those tests that feels really relevant here, unsure what's going on there. |
In conclusion:
On Linux and MacOS, both of the new test cases fail for this reason: |
@vexx32 @iSazonov I'm unable to get the tests to work after multiple attempts and speaking with people in the Powershell Discord. |
Eh, any change in Parser is very sensitive and nobody sign the change until we get clear understanding why the tests fail.
I suggest to pull new draft test PR with only the new your tests and without the code change and see CI results. |
Please see failures here: #18398 |
Not waiting on author. Waiting on experienced pester user input on broken tests. |
Ready to review? |
Yes! |
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
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.
One suggestion that I committed, otherwise LGTM
/rebase |
Moved $env:PATH restoration to AfterEach from AfterAll
15f4721
to
9bf9f7b
Compare
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
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.
0x0.ps1
doesn't work with the PR build. Do we expect this should be fixed in the PR too?
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment. |
PR Summary
In #15543 it was discovered that if you have a case where a script is named in a way that can be interpreted as hexadecimal (such as
0x.ps1
and you execute it without the dot backslash convention (.\0x.ps1
the script will always return null.@vexx32 identified the problem as being an oversight in an
if
statement that doesn't account for a number appended a few lines above.PR Context
This aims to solve #15543.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).