-
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
Get-Content -Raw should not neglects to read the last character #5076
Conversation
@mklement0 Could you please review? |
@@ -733,7 +733,7 @@ private bool ReadDelimited(bool waitChanges, ArrayList blocks, bool readBackward | |||
// (Trimming it during backward reading would not only be unnecessary, but could interfere with determining the correct start position.) | |||
string contentString = content.ToString(); | |||
blocks.Add( | |||
!readBackward && contentString.EndsWith(actualDelimiter, StringComparison.Ordinal) | |||
!readBackward && contentString.EndsWith(actualDelimiter, StringComparison.Ordinal) && !_isRawStream |
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.
This looks good;
but original issue says that "Note that Windows Powershell is not affected".
Any ideas why this does not repro in Windows PS?
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.
This is because the code is introduced by core ps code change.
@@ -285,3 +285,28 @@ baz | |||
} | |||
} | |||
} | |||
|
|||
Describe "Get-Content -Raw test" -Tags "CI" { |
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.
This test code is a good candidate to be rewritten using -testcases
construct, as per testing guidelines.
@anmenaga your comments are resolved. thanks |
@{character = "a`r`nb`r`n"; testname = "CRLF-terminated files"; filename = "crlf.txt"} | ||
@{character = "a`nb"; testname = "LF-separated files without trailing newline"; filename = "lf-nt.txt"} | ||
@{character = "a`r`nb"; testname = "CRLF-separated files without trailing newline"; filename = "crlf-nt.txt"} | ||
) |
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.
Please move to It
block:
It "Reads - <testname> in full" -TestCases @(
@{character = "a`nb`n"; testname = "LF-terminated files"; filename = "lf.txt"}
....
) {
....
}
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.
Thanks for your comment. But I would rather not change it as it is just format preference and the submitted format is following the testing guidelines example.
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.
It is new code not formatting. We should place it in a right place. The right place is either It
block (sample) or BeforeAll
block. We use the set only one time so It
block is best place. This does not contradict the testing guidelines.
@chunqingchen Thanks for the fix! |
resolve #4980
once the -raw parameter is set, we should force the contentString not to substring under any circumstance
Before the fix:
get-content -raw will ignore \n
After the fix:
get-content -raw will get all the contents include \n