-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Replace hardcode linefeed #11746
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
Replace hardcode linefeed #11746
Conversation
|
@PoshChan please retry macos |
|
@TylerLeonhardt, successfully started retry of |
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.
@rjmholt Any possibility this simple change could cause a regression?
| At line:1 char:5 | ||
| + $x = | ||
| + ~ | ||
| You must provide a value expression following the '=' operator. |
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.
Localized message is not good in test. We could remove it and check that right newline is after the extend.
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.
I've reworked the test.
I don't know of anything that depends on the |
Guess what, our error view depends on this |
🤦♂ |
|
🎉 Handy links: |
PR Summary
I've moved it over to using
Environment.NewLine. I added a test to test this specific thing... but maybe in the future we can put that test in a "crossplat" test file that checks all of ourToString's to make sure they honor the newline for the platform.PR Context
There was a hardcoded line feed in the
ToStringmethod ofParseError. this caused a CI test failure in the Jupyter kernel because on Windows, there would be all\r\nexcept for this single instance of\n.Tooling could be impacted, but from my look around the tooling code, I don't think it would be.
This is a breaking change, albeit small... and worth it IMO.
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.