GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Prettier formatting for ConvertTo-Json output. #2736 by kittholland · Pull Request #2787 · PowerShell/PowerShell

Conversation

Kitt Holland (kittholland) commented Nov 26, 2016 •

edited by joeyaiello

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fdwr reacted with thumbs up emoji

ryanwilliams83 reacted with thumbs down emoji

All reactions

👍 1 reaction
👎 1 reaction

Microsoft Contribution License Agreements (msftclas) commented Nov 26, 2016

Copy link

Copy Markdown

Hi Kitt Holland (@kittholland), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label

Nov 26, 2016

PowerShellTeam added the Review - Needed The PR is being reviewed label

Nov 28, 2016

Steve Lee (SteveL-MSFT) requested changes

Nov 29, 2016

View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link

Copy Markdown

Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Steve Lee (SteveL-MSFT) commented Nov 29, 2016

Copy link

Copy Markdown

Member

We should have a test for pretty print.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) approved these changes

Nov 29, 2016

View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link

Copy Markdown

Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kitt Holland (kittholland) commented Nov 29, 2016

Copy link

Copy Markdown

Contributor Author

I will work on the pretty print test soon.

SteveL-MSFT reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) approved these changes

Nov 30, 2016

View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link

Copy Markdown

Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Steve Lee (SteveL-MSFT) commented Nov 30, 2016

Copy link

Copy Markdown

Member

Dongbo Wang (@daxian-dbw) the CI breaks are related to #2806, how to proceed?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

James Truher (JamesWTruher) reviewed

Nov 30, 2016

View reviewed changes

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1 Outdated

James Truher (JamesWTruher) Nov 30, 2016

Copy link

Copy Markdown

Collaborator

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kitt Holland (kittholland) Dec 1, 2016

Copy link

Copy Markdown

Contributor Author

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.

Checking in fix now, thanks for pointing out the correct location.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) commented Dec 1, 2016

Copy link

Copy Markdown

Member

Steve Lee (@SteveL-MSFT) I just restarted the Travis CI build.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Kitt Holland (kittholland) commented Dec 2, 2016

Copy link

Copy Markdown

Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Jason Shirk (lzybkr) added Review - Waiting on Author and removed Review - Needed The PR is being reviewed labels

Dec 2, 2016

PowerShellTeam added Review - Needed The PR is being reviewed and removed Review - Waiting on Author labels

Dec 3, 2016

Kitt Holland (kittholland) force-pushed the ConvertTo-JSON branch from cde9dcf to 3606650 Compare

December 7, 2016 00:13

Kitt Holland (kittholland) added 4 commits

December 6, 2016 16:36


          Prettier formatting for ConvertTo-Json output. PowerShell#2736

cd4ace7

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


          Updated CoreCLR implementation to use NewtonSoft Indented Formatting

7e6a93b

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.


          Added tests that validate pretty Json output.

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 tests and removed extraneous file.

Moved pretty/compressed json tests from standalone file into the
existing ConvertTo-Json test file.

Kitt Holland (kittholland) force-pushed the ConvertTo-JSON branch from 3606650 to 9790487 Compare

December 7, 2016 00:37

Steve Lee (SteveL-MSFT) commented Dec 9, 2016

Copy link

Copy Markdown

Member

Closing and reopening to restart CI

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) closed this

Dec 9, 2016

Steve Lee (SteveL-MSFT) reopened this

Dec 9, 2016

Microsoft Contribution License Agreements (msftclas) commented Dec 9, 2016

Copy link

Copy Markdown

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label

Dec 9, 2016


          Updated tests for cross-platform support

c9a7510

Kitt Holland (kittholland) commented Dec 9, 2016

Copy link

Copy Markdown

Contributor Author

Can we retry the travis CI? I have successful test results on ubuntu 14.04.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) commented Dec 9, 2016

Copy link

Copy Markdown

Member

Closing and reopening to restart CI

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) closed this

Dec 9, 2016

Steve Lee (SteveL-MSFT) reopened this

Dec 9, 2016

Microsoft Contribution License Agreements (msftclas) commented Dec 9, 2016

Copy link

Copy Markdown

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label

Dec 9, 2016

Kitt Holland (kittholland) commented Dec 13, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mike Richmond (mirichmo) commented Feb 9, 2017

Copy link

Copy Markdown

Member

Steve Lee (@SteveL-MSFT) and James Truher (@JamesWTruher) This looks like a forgotten PR that is a candidate for merging. Can you please take a second look at it?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mike Richmond (mirichmo) self-assigned this

Feb 17, 2017

Steve Lee (SteveL-MSFT) approved these changes

Feb 24, 2017

View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link

Copy Markdown

Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mike Richmond (mirichmo) merged commit d69193e into PowerShell:master

Feb 24, 2017

Dongbo Wang (daxian-dbw) mentioned this pull request

Mar 8, 2017

Use prettier formatting for ConvertTo-Json #2736

Closed

Ilya (iSazonov) removed the Review - Needed The PR is being reviewed label

Mar 27, 2017

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Prettier formatting for ConvertTo-Json output. PowerShell#2736 (Power…

9654e0e

…Shell#2787)

* Prettier formatting for ConvertTo-Json output. PowerShell#2736

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

* Updated CoreCLR implementation to use NewtonSoft Indented Formatting

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.

* Added tests that validate pretty Json output.

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 tests and removed extraneous file.

Moved pretty/compressed json tests from standalone file into the
existing ConvertTo-Json test file.

* Updated tests for cross-platform support

Reviewers

Steve Lee (SteveL-MSFT)

SteveL-MSFT approved these changes

+1 more reviewer

James Truher (JamesWTruher)

JamesWTruher left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Mike Richmond (mirichmo)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Kitt Holland (kittholland) commented Nov 26, 2016 • edited by joeyaiello Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Nov 26, 2016

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 29, 2016

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Kitt Holland (kittholland) commented Nov 29, 2016

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) commented Nov 30, 2016

Uh oh!

James Truher (JamesWTruher) Nov 30, 2016

Choose a reason for hiding this comment

Uh oh!

Kitt Holland (kittholland) Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) commented Dec 1, 2016

Uh oh!

Kitt Holland (kittholland) commented Dec 2, 2016

Uh oh!

Steve Lee (SteveL-MSFT) commented Dec 9, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 9, 2016

Uh oh!

Kitt Holland (kittholland) commented Dec 9, 2016

Uh oh!

Steve Lee (SteveL-MSFT) commented Dec 9, 2016

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Dec 9, 2016

Uh oh!

Kitt Holland (kittholland) commented Dec 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mike Richmond (mirichmo) commented Feb 9, 2017

Uh oh!

Steve Lee (SteveL-MSFT) left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Kitt Holland (kittholland) commented Nov 26, 2016 •

edited by joeyaiello

Loading

Uh oh!

There was an error while loading. Please reload this page.

Kitt Holland (kittholland) commented Dec 13, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.