Allowing comments
Nobody has claimed this yet.
- Dominant language
- Scala
- Stars
- 436
- Forks
- 72
- Avg merge
- 1h 44m
- Merged PRs (30d)
- 1
Description
Hi Erik,
Very nice piece of work indeed!
Recently i changed from play to your parser for my home brew json operator lib. Because I have quite a lot if "comments" in my json i forked your lib, and wanted to extend it so it can spit out comments or at least skip them. Due to the clarity of your code this is rather easy to do.
I have a question about one design decision you made though. You differentiate between parseStringSimple and parseStringComplex, the only difference (from a performance point of view at least) seems to be the inspection of \\. The price you pay is rescanning the string when it turns out not be simple after all. Was this the sole ground for have two separate methods here, or am I missing something? Would one extra if statement in the scan make a big difference and not outweigh the disposal of work done?
If not, i would integrate the methods scan regularly until the first \\ and then, if this happens, switch to the collection into the CharBuilder. But, maybe you have already tested this and turned out to be a dead end.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with parseStringSimple, parseStringComplex, and CharBuilder, the entry points named in the issue, and compare how the fork handles comments. First settle whether comments should be emitted or skipped and which scanning design is preferred; done requires agreed comment behavior and a corresponding parser change.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100