-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
support for context.Render #21
Comments
I was planning to use I also plan to do opposite in c.Bind, so you can directly bind from JSON, XML ... to boost speed by not looking into content-type. |
@vishr I'm not sure auto content negotiation would be ideal. I was sort of expecting Render to render based on the c.Render(200, "person", &person{fname: "...", lname: ""}) RenderFunc can then read the template in Then anyone can go and override the import "github.com/unrolled/render"
r := render.New()
var e := echo.New()
e.RenderFunc = func(rw http.ResponseWriter, req *http.Request, statusCode int, name string, data interface{}) {
r.HTML(rw, statusCode, name, data)
}
e.Get("/", func(c *echo.Context) {
c.Render(200, "persons", &person{fname:"..", lname:".."})
}) |
In the above example it looks like e.RenderFunc is global, wouldn't it be a problem if I want to mix and match different renders? |
This was sort of what I was thinking of. e.RenderFunc = func(rw http.ResponseWriter, req *http.Request, statusCode int, name string, data interface{}) {
if strings.HasSuffix(name, ".md") {
r.Markdown(....)
} else {
r.HTML(....)
}
} Here is how one could render. e.Get("/", func (c *echo.Context) {
c.Render(200, "index", nil)
})
e.Get("/doc", func (e *echo.Context) {
c.Render(200, "doc.md", nil)
}) |
RenderFunc will be be serving all kind of formats. Now I was thinking how do we identify JSON or XML in the RenderFunc. |
That is where the I was thinking that The reason is the difference in api. Notice that render takes the c.Render(200, "doc.md", &model{})
c.Negotiate(200, &model{}) Some references: gin-gonic/gin#59 |
Also if you want to mix Here is an example in c#. Get["/"] = parameters => {
return Negotiate
.WithModel(new RatPack {FirstName = "Nancy "})
.WithMediaRangeModel("text/html", new RatPack {FirstName = "Nancy fancy pants"})
.WithView("negotiatedview")
.WithHeader("X-Custom", "SomeValue");
}; |
Just thinking out loud: In |
For c.HTML(200, `<div>Prabir Shrestha</div>`) But for c.Render(200, `<div>{{firstName}} {{lastName}}</div>`, &Person{"Prabir", "Shrestha"}) And it is the responsibitliy of the Also I don't think having Let says someone decides to use handlebars instead of the default template and they already have a bunch of existing templates. That person would want to easily update the |
So primarily this render function is for templates and html. Can you think of any other types? I think |
Yes. It is only for templates and html. Render is a pretty common terminology in the MVC/web world. So I think it suits better. Samples for
Samples for
I would prefer either |
Thanks for the detailed description and links. I will have a look at them and come up with an API to discuss further. |
@prabirshrestha This is what I have thought so far. What do you think? type RenderFunc func(io.Writer, string, interface{}) error
// Default RenderFunc
Echo.renderFunc: func(w io.Writer, name string, data interface{}) (err error) {
// Implement golang template/html and write it to w.
return
}
// Or provide your own RenderFunc?
func (c *Context) Render(code int, name string, data interface{}) error {
c.Response.Header().Set(HeaderContentType, MIMEHTML+"; charset=utf-8")
c.Response.WriteHeader(code)
return c.echo.renderFunc(c.Response.ResponseWriter, name, data)
} // API call
c.Render(200, "user.tmpl", &Users{"Joe"}) |
Looks awesome. Not related to this but would be awesome to have. Most likely a different issue. Since we now have a bunch of functions like
func serveXYZ(w http.ResponseWriter, r *http.Request) error { ... } |
renderFunc RenderFunc |
There is a function to set custom RenderFunc now 12bd049 |
@prabirshrestha I am planning to change RenderFunc to Renderer interface{} it's more idiomatic to Go. type Renderer interface {
Render(io.Writer, string, interface{})
}
func (c *Context) Render(code int, name string, data interface{}) error {
c.Response.Header().Set(HeaderContentType, MIMEHTML+"; charset=utf-8")
c.Response.WriteHeader(code)
return c.echo.renderer.Render(c.Response.ResponseWriter, name, data)
}
// API call
c.Render(200, "user.tmpl", &Users{"Joe"}) |
LGTM |
Signed-off-by: Vishal Rana <[email protected]>
@vishr Seems like you removed the status code. Can we have it back? Useful when rendering errors. func (c *Context) Render(name string, data interface{}) error {...} Also if I pass status code 0 I think it shouldn't write the head. Same for other methods like func (c *Context) Render(statusCode int, name string, data interface{}) error {
if c.echo.renderer == nil {
return ErrNoRenderer
}
if statusCode != 0 {
c.Response.Header().Set(HeaderContentType, MIMEHTML+"; charset=utf-8")
c.Response.WriteHeader(statusCode)
}
return c.echo.renderer.Render(c.Response, name, data)
} |
@prabirshrestha I have put it back - I completely missed on rendering errors. Verifying status code is nice but I then I think we should be doing it for all invalid codes - which is a lot of work, I believe, lets assume users are adults 😉 I looked into the URL you posted about handlers returning errors and I liked the idea. Now the default HandlerFunc returns error. I also added handlers which returns error to the list. 381fbae |
Would be awesome if there was a
Render
function in context similar to renderingJSON
.Having this
RendererFunc
interface would allow to use any renderer. Would also be good to havehtml/template
as a default Render. This is similar to howhttp.Handler and http.HandlerFunc
is implemented.gin use
... interface{}
fordata
. Also should we also pass*http.Request
toRender
so that it can support content negotiation? If we pass request asnil
then we can tell the render to not use content negotiation. Not sure if this is too much of magic. I think content negotiation should be a separate topic.Here is sample api that could be possible to use. Thoughts?
The text was updated successfully, but these errors were encountered: