Skip to content

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?

string contentString = content.ToString();
blocks.Add(
!readBackward && contentString.EndsWith(actualDelimiter, StringComparison.Ordinal)
!readBackward && contentString.EndsWith(actualDelimiter, StringComparison.Ordinal) && !_isRawStream

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.

}
}

Describe "Get-Content -Raw test" -Tags "CI" {

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

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