Skip to content
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

Merged
merged 2 commits into from
Oct 13, 2017

Conversation

chunqingchen
Copy link
Contributor

@chunqingchen chunqingchen commented Oct 10, 2017

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

@iSazonov
Copy link
Collaborator

@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
Copy link
Contributor

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?

Copy link
Contributor Author

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" {
Copy link
Contributor

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.

@iSazonov iSazonov self-assigned this Oct 11, 2017
@chunqingchen
Copy link
Contributor Author

@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"}
)
Copy link
Collaborator

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"} 
    ....
) {
    ....
}

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

@iSazonov iSazonov merged commit 07d3b18 into PowerShell:master Oct 13, 2017
@iSazonov
Copy link
Collaborator

@chunqingchen Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get-Content -Raw neglects to read the last character, if it is a LF
3 participants