Add streaming JSON parsing. #106
Conversation
Return the number of bytes processed from ParseStream. Add more tests.
src/json.cc
Outdated
| size_t total_bytes_consumed = 0; | ||
| yajl_handle& handle = state_->handle(); | ||
|
|
||
| for (;;) { |
There was a problem hiding this comment.
Would while (!stream.eof()) { be more concise?
There was a problem hiding this comment.
Yes, it would; also in line 446. It didn't use to be the case, but after an earlier series of refactorings of that code, the loop ended up being simple enough. Done.
| " \"ba" | ||
| )); | ||
| p.ParseStream(std::istringstream( | ||
| "r\": {\"x\": 0, \"y\": nu" |
There was a problem hiding this comment.
I had to spend a minute or two to reason about the indentation you've chosen. I don't think it's incredibly useful to preserve the location based on where you left off on the previous line. If you feel strongly about it, I'd say keep it, but otherwise, consistent indentations would have been easier for my brain to parse as you have the final json encoded check that makes it clear what the entire json blob represents.
There was a problem hiding this comment.
I wanted to make it clear that the stream literally stops in the middle of a token, which may not be obvious from the test name. I think it's important enough to call out, and indentation works better than comments, IMO. Let's stick with this for now — we can always do a cleanup pass later.
src/json.cc
Outdated
| } | ||
|
|
||
| size_t bytes = yajl_get_bytes_consumed(handle); | ||
| std::cerr << "Consumed " << bytes << " out of chunk " << count << " from '" << str << "'" << std::endl; |
There was a problem hiding this comment.
this is not on an error code path - why log to std::cerr?
There was a problem hiding this comment.
Oh, these were in an earlier commit, and were removed in the next one.
src/json.cc
Outdated
|
|
||
| size_t bytes = yajl_get_bytes_consumed(handle); | ||
| std::string str((const char*)data, bytes); | ||
| std::cerr << "Consumed stream " << bytes << ": '" << str << "'" << std::endl; |
There was a problem hiding this comment.
Oh, these were in an earlier commit, and were removed in the next one.
src/json.cc
Outdated
| size_t total_bytes_consumed = 0; | ||
| yajl_handle& handle = state_->handle(); | ||
|
|
||
| for (;;) { |
There was a problem hiding this comment.
Yes, it would; also in line 446. It didn't use to be the case, but after an earlier series of refactorings of that code, the loop ended up being simple enough. Done.
| " \"ba" | ||
| )); | ||
| p.ParseStream(std::istringstream( | ||
| "r\": {\"x\": 0, \"y\": nu" |
There was a problem hiding this comment.
I wanted to make it clear that the stream literally stops in the middle of a token, which may not be obvious from the test name. I think it's important enough to call out, and indentation works better than comments, IMO. Let's stick with this for now — we can always do a cleanup pass later.
src/json.cc
Outdated
| } | ||
|
|
||
| size_t bytes = yajl_get_bytes_consumed(handle); | ||
| std::cerr << "Consumed " << bytes << " out of chunk " << count << " from '" << str << "'" << std::endl; |
There was a problem hiding this comment.
Oh, these were in an earlier commit, and were removed in the next one.
src/json.cc
Outdated
|
|
||
| size_t bytes = yajl_get_bytes_consumed(handle); | ||
| std::string str((const char*)data, bytes); | ||
| std::cerr << "Consumed stream " << bytes << ": '" << str << "'" << std::endl; |
There was a problem hiding this comment.
Oh, these were in an earlier commit, and were removed in the next one.
|
@sparkprime LGTM'd offline. Merging. |
3 participants
No description provided.