Hello Kari,

thank you for your patch. I have reviewed it, but it is missing documentation. In general, it looks a bit over-engineered at the first sight. Why exactly is a new custom-status-t class needed? What is the use case that it supports? How does a

(defmethod hunchentoot:acceptor-status-message ((acceptor my-acceptor) status &key &allow-other-keys) ...)

or even several

(defmethod hunchentoot:acceptor-status-message ((acceptor my-acceptor) (status (eql +http-bad-request+)) &key &allow-other-keys) ...)

not suffice for your application? You can easily cover the ugliness of the specialization syntax with a trivial macro in your application. I'm not sure if the use case needs to be supported by fancier mechanisms in Hunchentoot itself, but maybe some other user here wants to speak up and support the case?

Please explain your design motivation and rationale a bit more so that we can understand why the added complexity is worthwhile.

Some source-level notes:

- Avoid abbreviations: define-make-... instead of def-make-... - An opening parenthesis is always preceded by whitespace - An opening parenthesis is never followed by whitespace - Docstrings are wrapped at 80 characters or less - Don't refer to other frameworks when explaining our features in documentation - Don't use ad-hoc reverse hungarian notation (custom-status-T) - Use generalized booleans unless there is a reason not to (when (>= status 400) t)) - Don't use PROGN without a reason - Long lines are not forbidden, but neither are shorter ones if it makes code easier to read

Sorry to have to reject your patch as it stands, Hans

On Wed, Feb 1, 2012 at 4:17 AM, Kari Lentz karilentz@att.net wrote:

Included is a patch that incorporates the ability to have a request specific reason phrase that allows one to use code such along the lines of

(setf (return-code*) (make-custom-status 500 "some database query error")))

This would override the templates and the cooked messages but only for that request and is intended to be an answer to the "StatusDescription" member of the HTTPResponse class in the ASP.NET library.  Essentially, a custom-status-t class has been added that links a code with a reason phrase.  The "make-custom-status" function above is just a constructor to this new class. The "reason-phrase" function is now a generic method that dispatches off both the custom-status-t class and the previous numeric status codes to be backward compatible.

I also ended up changing the error code dispatch to be CLOS based using eql dispatch instead of a case statement for the various http-status-code values.  It seems to be a trend to use generic functions instead of a case statement in this situation, probably because of a hash table type dispatch is used behind the scenes rather than going from start to finish as in a case statement.

Cheers,

Kari

---

tbnl-devel site list tbnl-devel@common-lisp.net http://common-lisp.net/mailman/listinfo/tbnl-devel