sheredom / json.h

🗄️ single header json parser for C and C++
The Unlicense
710 stars 79 forks source link

Partial support for simplified notation + fix unlikely bug #9

Closed ocornut closed 8 years ago

ocornut commented 8 years ago

Added support for "Quotes around object keys are optional if the keys are valid identifiers" Which ihmo is the important part of simplified json. I wonder if it could be under a #define and then the writer can assume that as well? Right now I haven't changed the writer.

Also that loop on line 435 would fail on a \\" pattern so I fixed that

  while (state->offset < state->size &&
    ('"' != state->src[state->offset] ||
    ('\\' == state->src[state->offset - 1] &&
    '"' == state->src[state->offset]))) {
    state->data[size++] = state->src[state->offset++];
  }

Became

  while (state->offset < state->size && '"' != state->src[state->offset]) {
    if ('\\' == state->src[state->offset]) {
      // copy reverse solidus character
      state->data[size++] = state->src[state->offset++];
    }
    state->data[size++] = state->src[state->offset++];
  }

Don't need to test for size again as we are on second pass. (Actually it became something else after I added non-quoted keys)

sheredom commented 8 years ago

I kind of wonder whether it would be better to have a parse function specifically for simplified json? It seems like a useful thing to be able to read/write, without affecting the current parsing/writing.

ocornut commented 8 years ago

It could but then you'd have so much more code to maintain.

sheredom commented 8 years ago

So I've added a new MR for adding new entry points for simplified json. Internally they reuse the same helpers as the normal json parsing does! https://github.com/sheredom/json.h/pull/10

I'll pull over your bug fix separately, but would be good to have you views on the other MR!

ocornut commented 8 years ago

My intuition - and perhaps we're talking matters of taste - would be to either add a set of flags (power-of-two-enums) passed to read/write functions (and minified can become a flag) so you have some flexibility ahead and less entry points, either move all of that to compile-time define.

Of course that depends how much you are willing or worried about breaking API. I would say it shouldn't be a problem.

Unrelated:

sheredom commented 8 years ago

Hm yeah! Maybe having a bitset of flags for various options would be nicer.

Naming things is hard, so at a stab what about;

1) json_flag_enable_trailing_comma - allow turning the trailing comma option on 2) json_flag_enable_non_string_ids - allow IDs like simplified JSON allows (non-quoted) 3) json_flag_enable_global_object - the whole json file becomes one big object

So for the void* instead of char* -> I wrote json.h immediately after writing utf8.h, and I thought that it would be better if json.h supported utf8 strings encoded in char* arrays. I've tried to write the code in json.h such that it should (in theory :D) just work with utf8 strings, and also be able to parse the json_string_s' data using the utf8.h functions.

ocornut commented 8 years ago

Would call them "allow" for those cases,

json_flag_allow_global_object json_flag_allow_traling_comma json_flag_allow_unquoted_keys

json_flag_simplified_json = json_flag_allow_global_object | json_flag_allow_traling_comma | json_flag_allow_unquoted_keys

unquoted? non_quoted?

Yes but utf8 by definition is compatible with char, it feels weird to type them as void.

sheredom commented 8 years ago

Agreed on allow then yep. Are you suggesting having a json_allow_simplified_json, which would just be set to the bitfield of the others? I kinda like that idea!

Sure, I'll take the action to change them to char* then :)

unquoted is slightly shorter so I'll probably stick with that.

ocornut commented 8 years ago

Yes; providing the json_flag_simplified_json shortcut would make sense to cover the majority of users. That's also how I feel about unquoted. Nice :)

sheredom commented 8 years ago

So I've done everything we talked about in this commit in https://github.com/sheredom/json.h/pull/12.

sheredom commented 8 years ago

So I've merged in my other branch that fixes all these issues I believe! Closing this one now.