frank icon indicating copy to clipboard operation
frank copied to clipboard

Match HTTP::Response params and return the output of the route

Open samsheff opened this issue 10 years ago • 5 comments

It seems the params of HTTP::Response#new have changed, and frank hasn't been updated. By removing the protocol header argument in the method call (which matches the HTTP lib), frank compiles and runs.

I also made a change that allows the body to return the output of the route, rather than a hardcoded "OK".

Edit: The second commit fixes an error where there are two response bodies returned.

samsheff avatar Sep 23 '14 04:09 samsheff

I also made a templating language similar to ERB that works with Frank, which is here: https://github.com/samsheff/Bunny

Let me know your thoughts on that as well! :)

samsheff avatar Sep 23 '14 06:09 samsheff

Hi @samsheff , did you know there is support for ECR in crystal?

I think there are no samples in the repo other that in the specs, but you can see the Changelog 0.2.0 or search for ecr_file macro.

The ECR works by generating a class so we have compile time checks of the template :-)

ecr source code

bcardiff avatar Sep 23 '14 12:09 bcardiff

Hey! your contribution came really fast after our chat in irc ;-)

It's true the HTTP::Response api changed but the change you did here is not correct. The "OK" wasn't the body but the response status message. Now the response initializer receives the status code (mandatory) then the body, headers, status message and http version (all optional hence usable with named arguments). Also the HTTP::Response class now have some helper methods ok, not_found and error to build common server responses. Please take a look to this class: https://github.com/manastech/crystal/blob/master/src/net/http/common/response.cr

Regarding the templating language (and as @bcardiff mentions as I'm writting this response :-P) please take a look to the ECR templates built in the std library in Crystal. However your approach have the advantage of not having to recompile the application every time you make a change in the template but ECR would be tons of times faster and more desirable I think, in a production environment.

waj avatar Sep 23 '14 13:09 waj

@waj @bcardiff Thanks for your feedback! In regards to ECR, I was looking for exactly that - But couldn't find it. I'll make corrections to the code I submitted to match @waj 's comments later tonight and you can let me know what you think. :)

samsheff avatar Sep 23 '14 13:09 samsheff

@waj @bcardiff Can you take a look at the latest commit and let me know if this is ok? It uses the helper methods and supports 404's. Also, I apologize if there needs to be additional work, I'm still learning my way around. :)

samsheff avatar Sep 23 '14 13:09 samsheff