Phrogz / NeatJSON

Pretty-print your JSON in Ruby, JS, or Lua with more power than JSON.stringify or JSON.pretty_generate
http://phrogz.net/JS/NeatJSON
MIT License
108 stars 19 forks source link

function based Sorting keys on multi-line object does not work without "short" #25

Closed bwl21 closed 7 years ago

bwl21 commented 7 years ago

22 solved the issue for sorting. But if opts.sort=function() ... it does not work, it always sorts alphabetically.

I will provide a test case

bwl21 commented 7 years ago

this test case shows that output is always sorted alphabetically by key.:

neatJSON({"b":17,"a":42,"c":20},{"wrap":1,"sort":"function (k){     return k=='a' ? 0 : k=='b' ? 2 : 1  }"})
EXPECTED
{
  "b":17,
  "c":20,
  "a":42
}
ACTUAL
{
  "a":42,
  "b":17,
  "c":20
} 

neatJSON({"b":17,"a":42,"c":20},{"wrap":1,"sort":"function (k,v){   return v                            }"})
EXPECTED
{
  "b":17,
  "c":20,
  "a":42
}
ACTUAL
{
  "a":42,
  "b":17,
  "c":20
} 

neatJSON({"b":17,"a":42,"c":20},{"wrap":1,"sort":"function (k,v){   return -v                           }"})
EXPECTED
{
  "a":42,
  "c":20,
  "b":17
}
ACTUAL
{
  "a":42,
  "b":17,
  "c":20
} 

neatJSON({"b":17,"a":42,"c":20},{"wrap":1,"sort":"function (k,v,o){ return v==o.a ? 0 : v==o.b ? 2 : 1  }"})
EXPECTED
{
  "a":42,
  "c":17,
  "b":20
}
ACTUAL
{
  "a":42,
  "b":17,
  "c":20
} 

105/109 tests passed in 51.00ms (2137 tests per second)
Phrogz commented 7 years ago

You are passing a string to the sort function. You must pass a function value instead. See the examples that start with "// JavaScript sorting examples" in the Options section of the README.

This appears to be a valid bug that appears when wrapping across multiple lines (but not in short mode):

var obj = {b:17, a:42, c:20};
var sorter = function(k){ return k=='a' ? 0 : k=='b' ? 2 : 1};

neatJSON( obj, {wrap:100, sort:sorter} );
// {"a":42,"c":20,"b":17}

neatJSON( obj, {wrap:1, sort:sorter} );
// {
//   "a":42,
//   "b":17,
//   "c":20
// }

neatJSON( obj, {wrap:1, short:true, sort:sorter} );
// {"a":42,
//  "c":20,
//  "b":17}
Phrogz commented 7 years ago

Thanks for the report and test case. As noted on your test case, two of your tests were faulty. I ended up fixing this using a simpler set of test cases, but I appreciate your work.

bwl21 commented 7 years ago

I replied to your findings there .

I deliberately made the test with three entries, and wanted the result not to be sorted alphabetically by the keys. If the result was alphabetically the test passed by accident as, in this case the sort was always alphabetical.

It is always a tradeoff to make the tests as simple as possible but on the other hand robust against side effects.

I have updated my testcases and all test pass now https://github.com/bwl21/NeatJSON/tree/feature/multiline_sort_wihtout-short

Not sure if you would like to incorporate into yours. So I did not create a PR.