| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
No problem with the XML changes, but concerned about opening the API to JSONML, JSONObject, and JSONArray to ignore type conversions. Is there any way to do this without changing the public API for those classes? |
Sorry, something went wrong.
|
JSONObject and JSONArray should be unaffected. JSONML is another XML representation and should be be consistent with the standard XML class I think. I'm personally unsure why JSONML is not a subclass of the XML class. On the surface, there appears to be a lot of duplicate code between the 2 but I didn't take the time to go through them and reduce duplication. |
Sorry, something went wrong.
|
Thanks for clarifying about JSONObject and JSONArray, agreed. I believe we have a certain freedom to work with XML, primarily because it was a candidate for deletion from the library. However, the same may not hold true for JSONML. Don't mind if they diverge a little, but unless there is a compelling argument otherwise, would prefer to avoid changes to the API that are not bug-related. |
Sorry, something went wrong.
|
Any existing code should still work as expected with JSONML. The caller would have to specify the new parameter with a true value to get the new parsing. I think the overall benefits of keeping the XML and the JSONML classes with similar APIs and features outweighs not implementing it. At the time of the original PR there also seems to be a decent amount of support for it. |
Sorry, something went wrong.
|
Just for reference, here are the other issues I found that reference XML improperly converting values that this PR may be a valid fix:
|
Sorry, something went wrong.
|
Thanks for doing the research, will need a little time to review those PRs. |
Sorry, something went wrong.
|
What problem does this code solve? Changes to the API? Will this require a new release? Should the documentation be updated? Change to unit tests? Note |
Sorry, something went wrong.
|
Adding this to bump the comments. This PR will be kept open for 7 days, in case XML or JSONML users want to comment. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
When reading XML and JSONML offer new option to not convert strings. This was mostly from PR #70 and is in reference for many issues brought up about conversions that are unwanted during XML processing, the lastest of which is #243
It expands on pr #70 by bringing the conversion option directly into the XML class and not just in JSONML. It also prefers polymorphism instead of new function names.
Test cases from the original PR will be included in a separate commit in the test project.