Open OsakiTsukiko opened 1 month ago
So, at the moment, I see it as a matter of taste. But if you can give me a compelling argument, I might reconsider "my taste" :-)
oh, it's just for ease of use, i needed a way to set all unused handlers to a 404 handler and setting all the other fields to my handler didn't occur to me for some reason only after making this :sob: but yea, its just so you can easily set a function to catch all unset handlers so you don't forget one or something. I guess it would be more useful if there were no nop and it would force you to handle all requests so that you don't have hidden behavior, for me at first I was expecting the main listener to catch this instead of the endpoint and I did not understand why it was a valid request but I didn't get any output, if this makes any sense.
Yes, it makes total sense! You make a convincing consistency-argument here!
My suggestion:
nop()
in there. But we make it a pub fn
, so you can provide .unset = zap.Endpoint.nop
at Endpoint.init..get = s.get // line-breaks just for GitHub comment
orelse s.unset
orelse @compileError("Endpoint handler `.get` is unset, and no `.unset` handler is provided.")
WDYT? Maybe we should even rename nop()
to nop_handler()
, to make its name more descriptive.
Sounds good. I'll get to changing it right away.
added the requested changes, went for @panic
instead of @compileError
, the former not forcing comptime and changed nop
to dummy_handler
.
Please elaborate. There already exists
nop()
for unset handlers. So this patch adds a kind of "and set all the other ones to this function here" - which I do not like. I like it to be explicit. It's 7 fields to set in total.