mustache

mustache copied to clipboard

This is bizarre because it must be intentional... which did you change Render and RenderFile to only return a string?

This goes against normal Go convention, which is when an Error is possible, have a return of (Whatever, os.Error), that way we can check to see if err != nil and handle errors that way.

Please fix this! You are a GREAT asset to the go community and it would be sad if one of Go's best programmers changed convention, because if you do, others probably will too :(

Hi Matthew,

It was done purely based on usage patterns. I looked at my code and realized that every single time Render or RenderFile was called, the return value was just ignored. This might seem like lazy programming, but in general once a template is written and then checked for errors, Render or RenderFile will simply never return an error. Furthermore, on the rare occasions when I actually did capture the error from Render or RenderFile (I think one case), I just printed the error string on the terminal. Because I use mustache.go to render web pages, this new interface allows me to simply see the error string on the web page itself.

What use case do you have where the error value is meaningful? Are you finding random and unexpected errors when dealing with these methods?

Furthermore, I'm a big subscriber of the DRY principle (don't repeat yourself), which is at odds with the way Go does error checking.

I've never encountered an error, however what you just said frightens me. I use mustache.go to render web pages as well. Do you want the error string in the browser in a production environment??? I don't! I want to capture the error, save it to a log, and render a backup "oops" page instead.

It's your library, I'll just check my diff when I update. It's only a couple of lines that need to be changed. Thanks anyways.

I suppose I could add MustRender and MustRenderFile which panics instead of returning the error. However in a production environment that would bring down the server instead of displaying the error message.

It would only bring down the web server if you pass the error to panic() instead of handling it in a different way, and I would presume someone wanting MustRender would want to handle it in a different way. E.g., throw the user a 500 error instead of a broken template? Or something more complex, depending on the task.

I do think it's important to have a way to programatically recognize when a template fails to render.

I'd accept a pull request that changes it back to returning an error. Is there one open?

I attempted to do this everywhere I found errors suppressed in this pull request, but I can't force a test program to error out by e.g., leaving out a variable I referenced in my template.

You can (slightly) work around this by using ParseFile that does return an error:

    tmpl, err := mustache.ParseFile(templateFileName)
    if err != nil {
        return "", err
    }

    text := tmpl.Render(model)

Render still has no error, but You can get some templating errors.

Or we all move to a language that actually has exceptions :unamused: sigh...