Remove FileStart and FileEnd events

[marler8997]

I see no use for the FileStart and FileEnd events. Moreover, they cause unnecessary code to be present in the user's application.

In DUB I have a foreach loop iterating over all the pull parser events. The dilemma I have is that I want to check every event type to make sure that I'm not missing any, but I have no need for the FileStart and FileEnd events. For example,

foreach(event; pullParser) {
    if(event.peek!FileStartEvent()) {
        // DO NOTHING, BUT I NEED THIS
        // SO I CAN THROW AN ERROR IN THE ELSE CLAUSE
    } else if(event.peek!FileEndEvent()) {
        // DO NOTHING, BUT I NEED THIS
        // SO I CAN THROW AN ERROR IN THE ELSE CLAUSE
    } else if(auto e = event.peek!TagStartEvent()) {
        // useful code...
    } else if(event.peek!TagEndEvent()) {
        // useful code...
    } else if(auto e = event.peek!ValueEvent()) {
        // useful code...
    } else if(auto e = event.peek!AttributeEvent()) {
        // useful code...
    } else {
        assert(false, "unhandled parser event!");
    }
}

I don't see why an application would ever need these events because if the application has code to execute at the beginning/end of the file, the code can be inserted before/after the for loop iterating over all the events. This method seems preferred anyway since it's more reliable to just have your own code before/after the for loop than to have to trust that the library is handling these events correctly.

Furthermore, having those events makes me think that maybe I should be verifying that I'm getting them as the first and last events of the parser but this provides no useful functionality other than making sure that your parser is behaving correctly. Having every application add extra code in order to ensure that the parser is handling the FileStart/FileEnd events are in the proper place just seems wrong.

[Abscissa]

I liked that FileStart/FileEnd meant that an SDL consumer could be defined purely as a mapping of events to delegates. Also, it means that if some sort of sub-consumer is passed a range of events, it can tell whether it's a full document or just a sub-section (which...may or may not ever matter...).

However, the natural (and arguably prudent) inclination to sanity-check that the parser outputs them correctly is admittedly a concern. Possibly a bigger concern.

I really don't think it's all that big of a deal though, any event you choose to not use is going to need to be explicitly handled in that loop.

I'll give this a little more thought and then decide.

(FWIW, another option for writing your loop is to popFront the first element, FileStart, and then go into the main loop. Then the FileStart handling could at least be omitted from the loop.

[Abscissa]

Ping @s-ludwig: What do you think of this idea?

[s-ludwig]

I'd generally prefer a tagged union (or TaggedAlgebraic ;-)) in this case, which would allow the use of final switch and would also reduce the syntax overhead of the additional two unused events, plus it's more efficient than the if-else-train. Apart from that, I think that only having the essential events makes sense overall, since everything else can easily be done by the user, if required.

[Abscissa]

Isn't std.variant.Algebraic a tagged union? I thought that was the whole point of it. Or do you mean something different? What's the difference between your TaggedAlgebraic vs std.variant.Algebraic?

[Abscissa]

Looking into it more, final switch has me sold on TaggedAlgebraic. Commit 6bf57c8 makes the switch to TaggedAlgebraic and removes the FileStart/FileEnd events.

Updated sample usage for the Pull parser is documented here. New docs will be generated when v0.10.0 is released (hopefully within the next few days).