Function skip_element about trial.protocol HOT 8 CLOSED

Given that json::reader is a kind of input iterator, I suggest that we model this after std::advance and/or std::next.

from trial.protocol.

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);

from trial.protocol.

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.

from trial.protocol.

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?

from trial.protocol.

or maybe _snode/_single_node/_one_node

from trial.protocol.

Instead of a suffix, we could also use a namespace like partial::skip and partial::parse.

from trial.protocol.

Okay, I like the namespace idea.

For a namespace name, partial works great.

from trial.protocol.

A neat use of json::skip()s return: https://gitlab.com/vinipsmaker/gawk-jsonstream/-/commit/243addd59ee3b4d2b8b8073df852a07614786ddb#b958235ee8db37bcaceda04b6d55553b450c2db4_141_148

from trial.protocol.