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.

Sign up for GitHub

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

Jump to bottom

Added the control to use self closing tag in XML module and added two unit cases to test the same. #754

Closed
wants to merge 1 commit into from
Closed

Added the control to use self closing tag in XML module and added two unit cases to test the same. #754

wants to merge 1 commit into from

Conversation

madhavgupta2011
Copy link

Added the control to use self closing tag in XML module and added two unit cases to test the same.

@madhavgupta2011 madhavgupta2011 mentioned this pull request Jul 10, 2023
Closed
@stleary
Copy link
Owner

stleary commented Jul 11, 2023

  • Tests are failing, please fix.
  • The default behavior should not change. Instead, try adding a property to XMLParserConfiguration to allow the developer to change the empty tag behavior. This object is passed to the private XML toString() method, so you don't need to add another parameter.

@madhavgupta2011
Copy link
Author

Sure. Will do it. Thanks @stleary

javadev
javadev reviewed Jul 24, 2023
View reviewed changes
src/main/java/org/json/XML.java
@@ -853,8 +870,15 @@ private static String toString(final Object object, final String tagName, final
sb.append(indent(indent));
sb.append('<');
sb.append(key);
sb.append("/>");
if(indentFactor > 0){
if (!keepSelfClosingTags) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (!condition) code1 else code2

can be replaces with

if (condition) code2 else code1

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, sure. I'll change it. I am thinking to change my approach though.

@javadev
Copy link
Contributor

javadev commented Jul 24, 2023 

[INFO] Finished at: 2023-07-11T12:12:16Z
[INFO] ------------------------------------------------------------------------
Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.3.2:testCompile (default-testCompile) on project json: Compilation failure
Error:  /home/runner/work/JSON-java/JSON-java/src/test/java/org/json/junit/XMLTest.java:[1325,33] error: no suitable method found for toString(JSONObject,boolean)

@javadev
Copy link
Contributor

javadev commented Aug 24, 2023

Since July 24, there have been no changes. Therefore, I believe it would be appropriate to consider canceling the pull request.

@stleary
Copy link
Owner

stleary commented Aug 24, 2023

@madhavgupta2011 Are you able to make the recommended corrections for this PR?
Do you think there is still a need for this change?

@stleary
Copy link
Owner

stleary commented Oct 5, 2023

Thanks, but not accepting this PR. The issue has been reopened.

@stleary stleary closed this Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javadev javadev javadev left review comments

Assignees
No one assigned
Labels
Assigned to Mel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@madhavgupta2011 @stleary @javadev