Conversation
…ions' into feature/5906-new-json-html-functions
|
Ready for a review :) |
|
|
||
| public class JsonHtmlFunctions { | ||
|
|
||
| JsonHtmlFunctions(JsonMTSTypeConversion converter) { |
There was a problem hiding this comment.
Can you add a @nonnull annotation to the argument to help IDEs pick up potential errors
| } | ||
|
|
||
| /** Track the current depth in the json hierarchy during the conversion to html table(s) */ | ||
| private Integer jsonPathDepth; |
There was a problem hiding this comment.
Since you are not using null as a "not defined" value this should be just an int to avoid all the auto boxing/unboxking every time you use it.
| optionSanitizeHtml = | ||
| options.has("sanitizehtml") | ||
| ? !options.get("sanitizehtml").getAsBigDecimal().equals(BigDecimal.ZERO) | ||
| : true; |
There was a problem hiding this comment.
Because all of this is user input you need to check they passed the correct type and raise a meaningfull error if they didn't, rather than the JSON library throwing a vague error they would not be able to debug.
Especially since the problems with MT Json not being able to use booleans have been addressed many versions ago, I would expect to be able to use boolean true/false instead of 1/0 in the options.
There was a problem hiding this comment.
Can take either boolean, or 1/0. Also added some option value validation and relevant exception messages.


Requirements for Contributing a Bug Fix or Enhancement
closes #5906
Description of the Change
New MTScript function
json.toHtmlTable(json [, options])which recursively transforms json into nested html tables with applied options.Possible Drawbacks
None foreseen, but added experimental flag while it matures.
Documentation Notes
To be done, but options are (in no particular order):
<img>elementRelease Notes
Add new MTScript function
json.toHtmlTable()This change is