Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chainable JSGI response helper #211

Merged
merged 4 commits into from
Feb 1, 2013
Merged

Chainable JSGI response helper #211

merged 4 commits into from
Feb 1, 2013

Conversation

botic
Copy link
Member

@botic botic commented Jan 25, 2013

This pull request targets issue #208 and tries to enhance the way to create a simple JSGI response. Instead of using a fixed set of function it provides a chainable JsgiResponse.

If you like this way, I will add unit tests in an update of this pull request. Here a short snipped showing how it works:

app.get("/test/text", function(req) {
   return response.text("Bad boy!").bad();
});

app.get("/test/html", function(req) {
   return response.html("<h1>Bad boy!</h1>").status(200).commit();
});

app.get("/test/json", function(req) {
   return response.json({"hello": "world"}).status(200).commit();
});

app.get("/test/headers", function(req) {
   return response.json({"hello": "world"}).headers({"x-foo": "bar"}).ok();
});

app.get("/test/redirect", function(req) {
   return response.redirect("http://ringojs.org");
});

app.get("/test/notmodified", function(req) {
   return response.notModified();
});

app.get("/test/static", function(req) {
   return response.static("./api/root.js");
});

@amigrave
Copy link
Contributor

I like those helpers !

I just wonder about something in the structure of the code.
Having methods of JsgiResponse defined on the fly prevent one to play with it's prototype and overload the helpers. Am I wrong ? I haven't dug the details so maybe I miss something.

@botic
Copy link
Member Author

botic commented Jan 30, 2013

I will re-factor the code to get rid of the helpers and of "committing" a request. There is no word about additional, non-enumerable properties in the JSGI response in the spec (JSGI Level 0/A Draft 2 Proposal), just "Middleware SHOULD preserve additional keys supplied in response object if not intending to supersede it".

@botic
Copy link
Member Author

botic commented Feb 1, 2013

I think this pull request is ready for merging into master. Any pleas or questions?

@amigrave
Copy link
Contributor

amigrave commented Feb 1, 2013

Cool, this is good stuff !
And since commit bc813d0 it's very flexible!

@grob
Copy link
Member

grob commented Feb 1, 2013

This pull request is a real beauty! Please merge :)

botic added a commit that referenced this pull request Feb 1, 2013
@botic botic merged commit 24f0105 into ringo:master Feb 1, 2013
@botic
Copy link
Member Author

botic commented Feb 1, 2013

Kudos to @grob and @oberhamsi for their input ;-)

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants