Repository navigation
Behavior of operator>> should more closely resemble that of built-in overloads. #367
Copy link
Copy link
Closed
Description
Basically:
- eat up leading blanks or newlines
- leave the stream where the parsing ended, allowing for more data downstream, which may or may not be json.
#include "json.hpp"
template<typename T>
static void test(const std::string& s)
{
T val{999};
std::stringstream sstr(s);
//sstr.exceptions(std::ifstream::failbit | std::ifstream::badbit);
try {
size_t i = 0;
while(sstr >> val) {
std::cerr << " result[" << i++ << "]=" << val
<< "; streampos=" << sstr.tellg() << std::endl;
}
std::cerr << " final values: result=" << val
<< " i=" << i
<< " streampos=" << sstr.tellg()
<< " streamflags=" << sstr.flags()
<< std::endl;
} catch(std::exception& e) {
std::cerr << " std::exception:" << e.what() << std::endl;
}
std::cerr << std::endl;
}
int main()
{
using json = nlohmann::json;
size_t i = 0;
for(std::string s : {
"",
" ",
"111",
"222 \t\n",
"\n\t 333",
" 111 \n222\n \n 333" })
{
std::cerr << "\nTest-case[" << i++ << "]: '" << s << "'" << std::endl;
std::cerr << "With int:" << std::endl;
test<int>(s);
std::cerr << "With json:" << std::endl;
test<json>(s);
std::cerr << "----" << std::endl;
}
return 0;
}Test-case[0]: ''
With int:
final values: result=999 i=0 streampos=-1 streamflags=4098
With json:
std::exception:parse error - unexpected end of input
----
Test-case[1]: ' '
With int:
final values: result=999 i=0 streampos=-1 streamflags=4098
With json:
std::exception:parse error - unexpected end of input
----
Test-case[2]: '111'
With int:
result[0]=111; streampos=-1
final values: result=111 i=1 streampos=-1 streamflags=4098
With json:
result[0]=111; streampos=-1
std::exception:basic_string::append
----
Test-case[3]: '222
'
With int:
result[0]=222; streampos=3
final values: result=222 i=1 streampos=-1 streamflags=4098
With json:
final values: result=222 i=0 streampos=-1 streamflags=4098
----
Test-case[4]: '
333'
With int:
result[0]=333; streampos=-1
final values: result=333 i=1 streampos=-1 streamflags=4098
With json:
result[0]=333; streampos=-1
std::exception:basic_string::append
----
Test-case[5]: ' 111
222
333'
With int:
result[0]=111; streampos=4
result[1]=222; streampos=9
result[2]=333; streampos=-1
final values: result=333 i=3 streampos=-1 streamflags=4098
With json:
std::exception:parse error - unexpected number literal; expected end of input
----
Activity
nlohmann commentedon Nov 24, 2016
The lines
std::exception:basic_string::appendrevealed a bug in thefill_line_bufferfunction wherewas executed with
n = 0. This must be fixed.About the inputs:
""(empty input): An empty input is not a valid JSON document. Hence, I think the messageparse error - unexpected end of inputis adequate." "(white space): After ignoring whitespace, this is again empty input, cf. input 1."111"(number): Parsing this input yields a number. Parsing the (then empty) input again, is the same as input 1."222 \t\n"(number with trailing whitespace): The input is parsed as a number while reading the trailing whitespace. Parsing the (then empty) input again, is the same as input 1."\n\t 333"(number with leading whitespace): Same as input 4." 111 \n222\n \n 333"(number with leading whitespace, followed by additional numbers): A number must not be followed by a number, so the messageparse error - unexpected number literal; expected end of inputmakes sense to me (note that after merging Exception line #301 this message will contain the byte offset of the error).I can understand your thoughts about the behavior of the streams. However, I think it is not a good idea to stop parsing once we found a JSON value, because this would mean that code like
json j; j << "false foo bar";would run without exception.
Am I too strict about this?
🐛 fixed a bug if parser was called with a stream at EOF (#367)
✅ added a test for EOF error (#367)
TurpentineDistillery commentedon Nov 24, 2016
An empty input is not a valid integer either, yet cin>>i will not throw - it will consume the blanks and leave i unchanged.
There are two classes of use-cases:
Think of a what kind of code a new user of the library would write, assuming they have not read any documentation other some examples, when they try to read a bunch of json values from stdin in a loop. Now make the API work that way, following the "design principle of least astonishment" : )
nlohmann commentedon Nov 24, 2016
I understand your point and you answered my "Am I too strict about this?" question with "yes" 😄
It would be nice to hear other opinions about this. I still feel uncomfortable accepting non-valid JSON files by just looking at a prefix...
gregmarr commentedon Nov 25, 2016
Seems reasonable for streaming to stop after a JSON document has been read. If this is a behavior change, it might require a major version bump.
Note that both
j << cin;andcin >> j;call the same code.I agree that this seems weird, but I'm not sure it's valid right now, since there is no operator<< that takes a string:
TurpentineDistillery commentedon Nov 25, 2016
Note for the future:
Suppose input stream contains arbitrarily large, or infinite number of json records (e.g. sensor data being piped from some upstream process), which happen to not be newline-delimited. calling getline() on such stream will fail.
mrkgnao commentedon Jan 13, 2017
+1, this is basically the only thing I wish for here. Is there any way to simulate this behavior with the library as it stands now?
nlohmann commentedon Jan 13, 2017
Unfortunately not.
gregmarr commentedon Jan 13, 2017
I saw someone do it in a way that required a two-stage parse. They parsed the data once, got the position of the parse error, assumed that was the start of the next whole entry, and so parsed again limiting to just before the parse error, and then moving the start position to the error position for the next entry.
48 remaining items
nlohmann commentedon May 21, 2017
In may add special overloads for ifstream and istringstream that use caches and remove caching for general istream cases. Would this work?
ceztko commentedon May 21, 2017
For me it's also a workable solution. You may also consider having the caching only on ifstream since istringstream it's already on memory.
🔨 trying something #367
✅ added test for #367
🔨 trying to fix #367
nlohmann commentedon Jun 12, 2017
@ceztko I openend a question at StackOverflow and found out the code I used to "rewind" the stream was not correct. I fixed it in a feature branch and the example from #367 (comment) is now working with MSVC 2015 and MSVC 2017 (see AppVeyor build).
ceztko commentedon Jun 12, 2017
@nlohmann great news, thanks! Sorry for not having looked at this: I wanted to debug it but I couldn't find the time.
nlohmann commentedon Jun 13, 2017
Merged fd4a0ec which fixes this issue. Thanks everybody for the patience.
Test to confirm parsing of multiple JSON records in a istream nlohman…
Test to confirm parsing of multiple JSON records in a istream nlohman…
Test to confirm parsing of multiple JSON records in a istream nlohman…
Test to confirm parsing of multiple JSON records in a istream nlohman…
Merge pull request #764 from pjkundert/develop-simplify-istream