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

Add a feature of loading templates from binary data generated by go-bindata #28

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yosssi
Copy link
Member

@yosssi yosssi commented May 12, 2014

I added a feature of loading templates from binary data generated by go-bindata.

I created martini-contrib/staticbin so that Martini can serve static files from binary data.

By adding this feature on martini-contrib/render, a web application using Martini can be compiled into a single binary file completely.

I would be glad if you could merge this PR.

[Changes]

  • Added the fields of Asset, AssetName on Options.
  • Cut out the inner logic of filepath.Walk to load function.
  • If options.Asset and options.AssetNames are set, load templates from binary data. Otherwise, load templates from files as before.

I thought of creating a new middleware which has this feature. But martini-contrib/render had various important features and I thought it was not good idea to create a new middleware similar to this one, so I decided to add this feature on the existing martini-contrib/render middleware.

Thanks.

@hypebeast
Copy link

Hi Yosssi,

I just added support for martini-contrib/staticbin to martini-contrib/render (https://github.com/hypebeast/render). Unfortunately, I didn't check the open pull requests for render and overlooked your work.

I tested your fork and it works quite well for me. Great work and thanks for the great staticbin library.

if options.Asset != nil && options.AssetNames != nil {
// Load templates from the assets (binary data).
for _, path := range options.AssetNames {
load(options, dir, path, t, options.Asset)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, it's a good idea to check if the asset is actually a template. Otherwise, non template files with the same extension as the templates gets compiled, too. It should be enough to check if the asset is part of the templates directory. Something like that:

if strings.HasPrefix(path, dir) {
    load(options, dir, path, t, options.Asset)
}

@yosssi
Copy link
Member Author

yosssi commented May 13, 2014

Hi @hypebeast,

Thanks for your comment and reviewing!

Maybe, it's a good idea to check if the asset is actually a template. Otherwise, non template files with the same extension as the templates gets compiled, too. It should be enough to check if the asset is part of the templates directory.

Oh, you are right and I didn't notice that! I fixed it.

Thanks for your valuable advice!

Hope that this PR will be merged and we will be able to use this feature soon.

@hypebeast
Copy link

I hope it to :)

@pboehm
Copy link

pboehm commented May 30, 2014

👍

@donovanhide
Copy link

👍

@yosssi
Copy link
Member Author

yosssi commented Jul 24, 2014

@pboehm @donovanhide Thanks!

@ghophp
Copy link

ghophp commented Nov 22, 2015

👍 merge please :D

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.

5 participants