Prettier formatting for ConvertTo-Json output. #2736#2787
Prettier formatting for ConvertTo-Json output. #2736#2787
Conversation
|
Hi @kittholland, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
SteveL-MSFT
left a comment
There was a problem hiding this comment.
On CoreCLR, I would prefer to use the NewtonSoft Json serializer with Format=Formatting.Indented. For FullCLR, I would just leave it as-is today. The compiler should leave out all the custom formatting code for a CoreCLR build, otherwise we can #ifdef it.
|
We should have a test for pretty print. |
|
I will work on the pretty print test soon. |
|
@daxian-dbw the CI breaks are related to #2806, how to proceed? |
There was a problem hiding this comment.
Describe [](start = 0, length = 8)
please add these to test\powershell\Modules\Microsoft.PowerShell.Utility\pester.commands.cmdlets.json.tests.ps1 rather than creating a new file
There was a problem hiding this comment.
Checking in fix now, thanks for pointing out the correct location.
|
@SteveL-MSFT I just restarted the Travis CI build. |
|
Thanks for rerunning Travis-CI, I see my tests are failing in that environment. I will have to build out a non windows environment for more testing. There are also some here-string based tests that are labeled -pending$isCoreCLR and are tagged in a Feature describe block. I think I will need to rework any here-string based tests to accomodate the new formatting. |
cde9dcf to
3606650
Compare
This change standardizes JSON output to example given, as well as
codemaid and online lint tools.
Sample object used for testing:
@{
foo = @{
first = 'a'
second = 'bbbbbbbb'
}
barbarbarbar = @{
first = 'a'
second = 'bbbbbbbb'
NestedArray = @(
'Test3'
'Test4'
'Test5'
3
4
)
NestedObject = @{
MoreObject = 'AnotherObject'
TestBool = $true
}
}
array = @(
'Thing1'
'Thing2'
)
dan = 15
} | ConvertTo-Json
I did not change the FullCLR behavior, I was not sure if you meant to revert my changes or to leave it as is in the current pull request.
Not sure if there is a better thought on how to implement these. The first two fail against current master, but succeed once this PR is applied. Third test is successful prior and post this PR.
Moved pretty/compressed json tests from standalone file into the existing ConvertTo-Json test file.
3606650 to
9790487
Compare
|
Closing and reopening to restart CI |
|
Hi @kittholland, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
|
Can we retry the travis CI? I have successful test results on ubuntu 14.04. |
|
Closing and reopening to restart CI |
|
Hi @kittholland, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
|
Due to differences in newline characters between platforms, I think doing comparisons on here-strings will have to be very carefully handled if it is possible. Currently these tests are marked with Feature tag and $pending:($IsCoreCLR) so I think its ok. |
|
@SteveL-MSFT and @JamesWTruher This looks like a forgotten PR that is a candidate for merging. Can you please take a second look at it? |
This change standardizes JSON output to example given, as well as
codemaid and online lint tools.
Sample object used for testing: