google / json5format

JSON5 (a.k.a., "JSON for Humans") formatter that preserves contextual comments
https://crates.io/crates/json5format
BSD 3-Clause "New" or "Revised" License
101 stars 18 forks source link

Consider using nom or another parser-support library? #5

Open anp opened 4 years ago

anp commented 4 years ago

The current regex parsing strategy looks like it works quite well and is very well tested, but I have concerns about the efficiency and maintainability of regex-based parsers. This (really awesome!) tool seems likely to be with us for a long haul so I'd encourage the authors to look into using a crate like nom or pest for handling the actual parsing. nom has a fairly concise example of using it to parse json which I imagine could be adapted to support json5 and the additional parsing done there.

richkadel commented 4 years ago

Thanks for the suggestion! I'm sure it's possible to use something like nom or pest to support the parsing, but the reason I didn't start with this approach was twofold:

A change like this is pretty major, and I don't think you are suggesting the result will provide any new capability. Given the current implementation works (and is well tested as you point out), I think it's safer to leave this as-is.

Let me know if you have other thoughts.

Thanks for reviewing!