Invert set_pre_routing_handler return value semantics

[ghost]

Currently, I feel like Sever::set_pre_routing_handler's return value is inverted, especially when compared to how Server::Get and such operate.
true regularly means to continue the process, while false means to cancel it, but set_pre_routing_handler does the opposite; true cancels the process, false continues on to the "normal" handler.

Technicially, this would be a breaking change, but since it's been here for only 15 days, maybe it wouldn't be too late?

[yhirose]

@00ff0000red, in fact, I intentionally made the behavior. The content reader for Get could be called a number of times by chunk until all the incoming data is read. true from the reader means please continue to get another chunk, and false means something happened, please stop sending remaining data.

Instead, the pre routing handler is called only once. true simply means the request has been handled, please skip the regular routing process, and false means didn't get handled, please let the regular routing process handle the request.

I still feel that the current behavior is more natural for me. But if most of users think differently, I don't mind reconsidering this matter. But first, I would like to do more research how other http libraries and frameworks are treating it.

Anyway, thanks for bringing this thing up!

[ghost]

@yhirose I think I understand the root of the problem, it's that booleans themselves don't have any semantic meaning on their own.

In a situation like this, I think that an enum would make much more sense, as it would attach a name to the value, and associate semantics with it.

I'm thinking something like so:

enum class PreHandlerResponse : bool {
    Handled,
    Unhandled
};

Updating the example in the README, we have this:

using httplib::PreHandlerResponse;

svr.set_pre_routing_handler([](const auto& req, auto& res) -> PreHandlerResponse {
  if (req.path == "/hello") {
    res.set_content("world", "text/html");
    return PreHandlerResponse::Handled; // This request is handled
  }
  return PreHandlerResponse::Unhandled; // Let the router handle this request
});

IMO, that makes that snippet much more readable, as opposed to the "random" true and false which could mean anything, whereas here it's quite obvious, even without the comments as to what everything means. I feel like this could've been done for more of the API, but it's too late now. Maybe after it's frozen and you've moved on to the next iteration?

I believe that an enum would nicely solve this situation, removing ambiguity and the differences in our interpretations, what do you think?

[yhirose]

@00ff0000red, sounds very good. I implemented it. Thanks!

[ghost]

@yhirose CI is currently failing after that commit.