Render option to sort keys #557
Conversation
|
Hi @mmolimar, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
|
Note: I have signed CLA. |
| @@ -64,7 +69,7 @@ public ConfigRenderOptions setComments(boolean value) { | |||
| if (value == comments) | |||
| return this; | |||
| else | |||
| return new ConfigRenderOptions(originComments, value, formatted, json); | |||
| return new ConfigRenderOptions(originComments, value, formatted, json, new DefaultComparator()); | |||
There was a problem hiding this comment.
These setters should be copying the existing comparator I believe. (otherwise setting anything other than the comparator would reset the comparator.) Meaning change new DefaultComparator() to comparator for all the set* methods.
| @@ -174,9 +204,53 @@ public String toString() { | |||
| sb.append("formatted,"); | |||
| if (json) | |||
| sb.append("json,"); | |||
| if (comparator != null) | |||
There was a problem hiding this comment.
is comparator actually allowed to be null? It looks to me like it isn't (it has to be DefaultComparator if not set explicitly)
| boolean aDigits = isAllDigits(a); | ||
| boolean bDigits = isAllDigits(b); | ||
| if (aDigits && bDigits) { | ||
| return Integer.compare(Integer.parseInt(a), Integer.parseInt(b)); |
There was a problem hiding this comment.
I know you're copying this from our existing code, but I think this will throw if the digits make an integer greater than maxint. We probably ought to fix that... (in its own commit and maybe own PR...)
There was a problem hiding this comment.
You're totally right. I've changed that to BigInteger to avoid an exception.
There was a problem hiding this comment.
I believe I am hitting the exact same issue described here while rendering some config. Is this change part of the latest 1.3.3 release? I don't see it on the milestone page yet.
Exception in thread "main" java.lang.NumberFormatException: For input string: "10000951726339"
at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
at java.lang.Integer.parseInt(Integer.java:583)
at java.lang.Integer.parseInt(Integer.java:615)
at com.typesafe.config.impl.SimpleConfigObject$RenderComparator.compare(SimpleConfigObject.java:452)
There was a problem hiding this comment.
I see it is a candidate for the next release. Please ignore the previous comment.
|
The idea to implement this is to allow the API to sort the keys in the order you like setting your custom comparator. In my case, I have to sort these keys in a specific order so that the rendered output has the same output as a template I have expects. |
|
We do not intend to extend the functionality of "Typesafe Config" further. |
No description provided.