Closed vinipsmaker closed 6 years ago
Given that json::reader
is a kind of input iterator, I suggest that we model this after std::advance
and/or std::next
.
Actually I was thinking something in the lines of:
boost::string_view json::skip_element(json::reader&);
skip_element would call literal()
on the first token to store the begin iterator/pointer and call literal()
on the last token to store the end iterator. Then it'd construct the string_view and return it. There is no need for error report in this function because the caller will just reader.symbol() == json::token::symbol::error
to check for errors.
I told you before about my need for a skip_element
to properly support arrays in piece of sotware A. For this need, I don't need the returned string_view
. I'm now talking about another need. So, there is piece of software B which receives a super JSON object. B's JSON doesn't need to be handled fast, so I just use dynamic::variable
to parse it. One of the fields is _po
which contains the JSON object which should be parsed with piece of software A.
So, I cannot mix the two parsers, because piece of software B doesn't use json::reader
directly. But suppose I was using it directly. In this case, I wouldn't be able to call the parsing function defined in piece of software A because it will consume reader
until the very end:
With skip_element
returning the JSON literal of the skipped area, I'd be able to create a new reader and feed it to piece of software A. I guess this is just the selling point of pull parsers, complete freedom to combine algorithms. It's not possible to do this outside of skip_element
because between the last token skipped and the next token, a comma might appear and an invalid JSON would be constructed (i.e. if you try to get the literal for the first token yourself and then get the literal after the skip_element
function returns).
At first, I don't like the distance
argument from std::advance
and std::next
because — at first — I wouldn't know what it would mean here.
Here is a small parsing algorithm that I have:
TestRequest parse_test_request_message(boost::string_view raw_json,
DynamicDictValidator &validator) {
// ...
json::reader reader(raw_json);
if (reader.symbol() != json::token::symbol::begin_object)
throw Error("bad object");
bool read_req_id = false;
// ...
TestRequest msg;
HeaderParser header_parser(msg.header);
if (!reader.next())
throw Error("bad object");
while (true) {
// Key
if (reader.symbol() == json::token::symbol::end_object) {
reader.next();
break;
}
assert(reader.symbol() == json::token::symbol::string);
auto current_key = reader.literal();
current_key.remove_prefix(1);
current_key.remove_suffix(1);
if (!reader.next())
throw Error("bad object");
// Value {{{
try {
if (current_key == "TestReqID") {
msg.header.req_id = reader.value<decltype(msg.header.req_id)>();
read_req_id = true;
} else if (/* ... */) {
// ...
} else {
if (!header_parser.consume_root_field(current_key, reader,
validator)) {
throw Error("bad object");
}
continue;
}
} catch (const json::error&) {
throw Error("bad object");
}
if (!validator.consume_root_field(current_key, reader))
throw Error("permission denied");
// }}}
}
if (reader.symbol() != json::token::symbol::end
|| !(read_req_id && /* ... */)
|| !validator.is_document_valid()) {
throw Error("bad object");
}
return msg;
};
The value block is where I handle values. Places like this are the places where I have a use for skip_element
. What would it mean to add a skip_element(reader, 2)
there? It'd skip the element and then skip one key of the object? After I wrote this message, I understood what would be the proper behaviour, then I'm not against it. Sorry about the confusing message, but I'm not gonna change it because I think it's a little informative. Anyway, I guess we're then for this interface:
string_view skip_element(json::reader&, std::size_ distance);
But I think it combines too many responsibilities, so I prefer two different functions (the latter can be implemented in terms of the former):
string_view skip_element(json::reader&);
void advance(json::reader&, std::size_t distance);
I like the idea of returning the skipped part.
I also agree that skip and advance should be split in two (and we can postpone advance until it is actually needed.)
I do not find the _element
suffix that intuitive. An alternative may be _single
, but my current preference is to simply call the algorithm skip
.
I like the idea of returning the skipped part.
Great.
we can postpone advance until it is actually needed
Okay.
I do not find the
_element
suffix that intuitive.
_node
maybe?
or maybe _snode
/_single_node
/_one_node
Instead of a suffix, we could also use a namespace like partial::skip
and partial::parse
.
Okay, I like the namespace idea.
For a namespace name, partial
works great.
Explained at https://github.com/breese/trial.protocol/issues/21#issuecomment-365286469