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

#459 Fixing indendation of printing JSON objects #460

Closed

Conversation

anandvarkeyphilips
Copy link

@anandvarkeyphilips anandvarkeyphilips commented Jan 19, 2019

An attempt to enhance existing pretty print functionality.

Copy link
Contributor

@johnjaylward johnjaylward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this add new lines to the compact output?

@anandvarkeyphilips
Copy link
Author

Does this add new lines to the compact output?

@johnjaylward No, I had missed that scenario. I have updated the code to handle it now. Please review and accept. :)

@stleary
Copy link
Owner

stleary commented Jan 20, 2019

@anandvarkeyphilips -

  • You did not modify JSONArray.java. Are you omitting new indent handling in arrays for a reason?
  • Breaks unit tests in JSON-Java-unit-test

@anandvarkeyphilips
Copy link
Author

@stleary , I didnt feel it was required. All appeared good.
Can you test here and advice on what to do?
http://varkeys-rhel-jenkins.westus.cloudapp.azure.com:8090/editor

@stleary
Copy link
Owner

stleary commented Jan 21, 2019

@anandvarkeyphilips Thanks for adding this enhancement to formatted output. Here is a more complete explanation of what still needs to be done.

  • JSONArray formatting may not be important for your use case, but consistent handling of JSON objects and arrays is usually a good idea, and others may find it helpful as well. It sounds like you did not specifically omit updating the JSONArray code. In that case, please consider making the same change to both classes, or explain why the JSONArray code should not be changed.
  • Please check the FAQ for unit tests. Your changes have broken the unit tests, and can't be accepted until the tests are fixed. No exceptions on this rule. The changes needed are straightforward, but you will need to submit a pull request on the unit test repository, which will be committed along with this change. Let me know if you have any questions about how to proceed.

@stleary
Copy link
Owner

stleary commented Mar 20, 2019

Closing due to no activity in the past 2 months.

@stleary stleary closed this Mar 20, 2019
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.

None yet

3 participants