search

zserge / jsmn

Jsmn is a world fastest JSON parser/tokenizer. This is the official repo replacing the old one at Bitbucket
MIT License
3.67k stars 781 forks source link

Modernize #149

Closed zserge closed 5 years ago

zserge commented 5 years ago

After years of inactivity, I decided to keep jsmn up-to-date with the modern expectations of how a tiny library should look like.

So far API has not been changed, but jsmn is now a single-header, header-only library, with fixed formatting rules and a linter (that already complains a lot).

zserge commented 5 years ago

@pt300 Hello, and pleased to meet you. You seem to be the most active commenter in the issue tracker and de-facto maintainer, so I would like to ask you to review it. Sorry for being silent for too long.

pt300 commented 5 years ago

I'd say that's an interesting approach. I'm myself not a fan of putting code inside header files as that's not the place for it, but in this case it actually seems to work nicely with JSMN considering it's intended use case and size. Doing it like that already throws out the window problem of binary incompatibility revolving around the JSMN_PARENT_LINKS as users are forced to compile JSMN with their project. Moreover it better fits the usual workflow when programming devices like 8bit microcontrollers where libraries are compiled along with main code instead of being used as separate precompiled binaries.

My thought on that for now is that it's not that bad of an idea but I have mixed feelings about it. Would like to hear other opinions.

rspencer01 commented 5 years ago

In my (limited) experience, header-only libraries are great and semistandard now for small libraries. The format means that they are very easily integrated into existing projects (don't need to worry about compiling and linking them in) and any tiny tweaks that the project may require of the library are easily implemented.

If you want an opinion, here is one: do it. :smile:

pt300 commented 5 years ago

Considering this change will be quite big I'd suggest going over currently open pull requests and consider incorporating changes such as #142 or #143

circulosmeos commented 5 years ago

Hi! I've just used the modernized version (flawlessly) in my project :+1:

Nelson-numerical-software commented 5 years ago

Hi, do you plan to release an new version ? Thanks

pt300 commented 5 years ago

What do you mean?

Nelson-numerical-software commented 5 years ago

Current version with tag in github: v1.0.0 This good feature requires a new tag :)

pt300 commented 5 years ago

touché

pt300 commented 5 years ago

I'll wait for @zserge to decide how exactly this should be handled.

zserge commented 5 years ago

@pt300 Since we don't have any "version" variables/macros in the actual header file or elsewhere in the sources - the only versioning we have is a git tag. So, let's put a tag then. To me this PR looks like an API-compatible change (all function signatures and data structs remained very much the same). So my suggestion is to make a v1.1.0 tag on the current HEAD.

pt300 commented 5 years ago

But the way how library is used changed. I think that should be taken into account.