harningt / luajson

JSON parser/encoder for Lua Parses JSON using LPEG for speed and flexibility. Depending on parser/encoder options, various values are preserved as best as possible.
http://www.eharning.us/wiki/luajson/
Other
251 stars 48 forks source link

Create LuaJSON json.util documentation #26

Closed agladysh closed 11 years ago

agladysh commented 11 years ago

ORIGINALLY: json.util.merge stopped working as expected

LuaJSON lacks useful documentation of utilities, even though it recommends the usage of them for customization. Documentation is a required attribute of useful software tools, and as such, LuaJSON needs it.

https://github.com/agladysh/json2lua/blob/master/json2lua#L171-L179

 decode_options = json_util.merge(
    {
      object =
      {
        setObjectKey = json_decode_util.setObjectKeyForceNumber;
      };
    },
    decode_options
  )

This code does not work (option is ignored) with 1.3, and it did work with 1.2. I did not find anything related in release notes, so I assume that this is a regression.

harningt commented 11 years ago

Hm, it looks like the merge logic may be inverted in the code.

It would have worked in 1.2 since object.setObjectKey was not defined in the 'simple' table. In 1.3, you can lookup what the actual options are now, but this has the side effect that it overwrites your option.

Example output for each version for json.decode.simple:

1.2 == just has the options that took the "defaults" to be the simple state

json.decode.simple = {
  others = {
    null = false,
    undefined = false
  }
}

1.3 == has a fully independent option state that will not be merged later with the "defaults"

json.decode.simple = {
  number = {
    hex = false,
     .....
   }
}

What you'd want there is the following construct to merge, I think:

merge({}, decode_options, {object = setObjectKey......})

That way you merge onto an empty table, first filling with options from decode_options, then filling from your {object = ...} one.

agladysh commented 11 years ago

Hmm, OK, thanks. Shouldn't this be documented somewhere?

harningt commented 11 years ago

Yes indeed, reworking this ticket as a documentation one.

harningt commented 11 years ago

Fixed as of release 1.3.2 - specifically commit 3883db4b5f24cb685af7998dbe38e18ca7ff54ff