Extending template engine support #152

Merged
merged 50 commits into from Jun 29, 2012

4 participants

@larzconwell

@Techwraith suggested I make a PR to discuss adding templates engines to Geddy

I figured this is a good start by identifying more template extensions.

What templates should we support? Or require as a default(Assuming EJS)?
I was thinking of a couple, EJS, Jade, CoffeeCup, Handlebars.js.

Also I was thinking an easy way to identify them was by their extension instead of having the user manually add some config object to identify the template engine.

@Techwraith

Definitely a good place to start here.

I'm thinking we want to support these"

  • EJS (default, also, don't use our hacky version, use a more accepted one)
  • Jade
  • Handlebars (.hbs)
  • Mustache (.mustache)

You can add in coffeecup support if you'd like, but I'm not sure how often it will be used. :)

@johnnygreen

.mustache gets a bit long, could we also support .ms for mustache? Not sure that you are suggesting limiting template filename by extension, just thought I would mention it. Maybe a way of specifying extension?

@Techwraith

@johnnygreen I would not be opposed to using both. Does .ms conflict with any other formats that you know of?

@larzconwell

I don't think it conflicts with any other formats, I'm not 100% sure though. (3DS Max uses it)

Would there be a better way to identify template engines without having them specifically putting somewhere what it is?

I actually like CoffeeCup but I think we'll see how difficult it'll be first then decide to put it in there or not (;

@johnnygreen

I used a kohana php bundle for mustache a few months back that supported both .mustache and .ms. Not sure if it's a standard.

@Techwraith

I actually like using the extension to try to figure out which template engine to use. Lets continue down that road for now.

@larzconwell

@johnnygreen Yeah I don't think it's standard but more generalized extension, It's certainly a lot easier to use.

@larzconwell

Okay I added a commit.

Geddy now loads the pages for other engines(I haven't got them rendering yet).

I also included a new Jade example app, but I'm not sure if it's correct yet as it's not rendering (;

To actually start rendering I was thinking about changing the lib/template directory to remove the adapters dir and make a script that will just have a render function that'll take all the normal data and an extension then run try to require an render that view.

I'm gonna search through the code in the template directory more tomorrow so I better understand what's being called on exactly (:

@Techwraith

Sounds ok to me. @mde was the one who did that stuff previously (you can tell 'cause he actually wrote his own EJS lib) ;)

@larzconwell

Yeah I saw, I thought that was pretty awesome!

@larzconwell

I rewrote the template directory so it should be somewhat easier to manage the different templating engines, I'm still working on it so sometime tomorrow I'm gonna try and plug in the actual EJS module and test how it works!

Currently the branch isn't running because when I merged @mde's new update either I messed up my merge or he forgot to add the lib/response/format module (; So when that get's fixed it should be running!

@mde
mde commented Jun 4, 2012

Yes, sorry for the big change, but in the long run it will make things a lot better. Now the 'html' format and template-rendering isn't that crazy-pants special-case reentrant thing. All the format handlers take a callback, so they can all be treated exactly the same.

I'm super-excited to see this work happening. It's something people have wanted for quite a while. Good stuff!

@mde
mde commented Jun 4, 2012

Oh, no, it's not you -- I totally forgot to add that file. It's added and pushed. Sorry, man. :)

@larzconwell

Don't be sorry! It's a lot better now, I like the new changes the callback idea is great.

Hopefully it won't be too hard to render the different engines, since they each use different ways to use partials and layouts.

Should we include EJS(https://github.com/visionmedia/ejs) as a dependency since it'll be a default?

@larzconwell

Thanks!

@larzconwell

Wow sorry for the mangled commit list guys, I'm not so good with updated, rebasing/merging and pulling in Git.

@mde
mde commented Jun 4, 2012

Just pushed a fix. Nice catch. We need better tests!

@mde
mde commented Jun 4, 2012

Oh, and I realize @Techwraith is keen to replace our current EJS impl. :) But unless some other one is demonstrably better in some way, or there are a bunch of bugs reported for ours, I don't really see the need. There are like four competing versions out there.

@Techwraith
@larzconwell

Thanks for that fix @mde, that way was much better than mine haha

What is the best way to update my branch with changes from the remote?

# HACKING HACKING
git commit -a

git checkout master
git pull --rebase geddy master

git checkout extending_template_engine_support
git rebase master

git push origin extending_template_engine_support

Is that right? I don't want to muddy up the logs anymore than I have already!

How about this! I create a wrapper for EJS that will try and require the module(npm install ejs) then catch that with our custom fallback?

@Techwraith
@mde
mde commented Jun 4, 2012

I'm not sure how you'd do all that with rebase. I constantly push to remote, so rebasing is limited value.

It's not quite as simple as doing a conditional require for EJS like we do for other, optional dependencies. EJS isn't optional. Geddy relies on it to for all the app and scaffold generators. This is another reason I'm not keen on adding an external dependency for this.

@Techwraith what bugs have you run into, and did they get issued? I fixed during JSConf, and added default-escaping during that Yammer hackathon, but that's all I know of. So far I'm not hearing any compelling technical reasons to switch.

@Techwraith
@Techwraith

Oh yeah, and should generators be part of this pull request? It would be really cool to be able to do geddy app foo -t jade or something and have the generator use Jade instead.

I know this adds a ton more work, so it's up to you @larzconwell.

@Techwraith

@mde also, as an argument against us using our own ejs lib - Do we really want to maintain a templating engine as well as a web framework? That seems a little outside the scope of Geddy's mission. If we use a module from the community we don't have to support it as much (just the integration points).

That alone is a good enough argument for me, but it's up to you on this one.

@larzconwell

@Techwraith After I got this PR in, I was gonna make another for making the Jade and Coffee generators!

I like the fact that we don't have to rely on external dependencies for ejs but at the same time I don't think Geddy should maintain it's own version of it, how about we use the single ejs module(https://github.com/visionmedia/ejs/blob/master/ejs.js) and include it in /deps so that way we're using an official version but we still don't have to worry about external dependencies?

@larzconwell

I'm running into some problems getting other engines to render and getting ours working together.

I'm not sure how to implement it so that all engines will work together, because our engine uses the yield function so that it'll inject the template content into our layout, but Jade uses it own layout syntax using extends and includes for partials.

So I'm not sure if we should use our partial and yield functions or if we should use the engines versions, which wouldn't be as flexible or consistent. But I feel if we use our functions then the developers would become confused because we aren't using the engines native inheritance. Also I'm not sure we can use our functions in all the engines.

Currently this is the code that is rendering the full view in EJS(lib/template/index.js line 42)

Templater.prototype.render = function(data, config) {
  // Rendering a layout
  if(config.layout) {
    this.isLayout = true;
    this.templateRoot = getDirName(config.layout);

    var self = this
      , templaterContent = new Templater()
      , contentPartial = '';

    // Add incoming data to content buffer
    templaterContent.addListener('data', function(content) {
      contentPartial += content;
    });

    templaterContent.addListener('end', function() {
      data.yield = function() {
        return contentPartial;
      };

      self.partial(getFileName(config.layout), data);
    });
    templaterContent.render(data, {template: config.template});
  }

  // Rendering a template
  else {
    var filename;
    this.templateRoot = getDirName(config.template);
    filename = getFileName(config.template);
    this.partial(filename, data);
  }
};

When you input it, it takes a config arg, so let's say the config is:

config = { 
  layout: 'app/views/layouts/todos',
  template: 'app/views/todos/index',
  controller: 'Todos',
  action: 'index'
}

This is the usual config used when routing the views.

First it checks if it has a layout object, if so it sets it as a layout, then calls the render function again using the same data and {template: 'app/views/todos/index' } as the config, then it creates the partial from that config.template file name, then it adds the content to contentPartial and returns that in the yield function(Which get's eval'ed from the ejs engine) then finally creates a partial out of all of that, which then gets returned to be viewed.

So it's actually going through the render function twice(or more), once for the template, and another for the layout. This works perfectly for our ejs engine, but using Jade(With extends for layout extension) since it goes through twice(the template being the first time, and layout the second time) it will render the entire template(layout included) the first time, then go around again and render only the layout, which then gets returned but only the layout is returned since it is the last to get rendered and doesn't use the yield function(plus it's not getting eval'ed so I'm not sure it'll work anyway) to inject the template in the layout.

I'm not sure how easily that is to read, but I tried! (;
So now I'm completely lost on a way to implement it, but I'd love some pointers!

@Techwraith

It sounds like we might have to modify how the rendering system works per rendering engine. That kinda sucks, but it seems like the only way I can think of to accomplish what we need to here.

@mde Any other suggestions?

@mde
mde commented Jun 7, 2012

Yes, I expected we'd have to have separate implementations (adapters) for each type of template. I tried to make the interface in the BaseController fairly simple -- see renderTemplate in base_controller.js. It's an EventEmitter that emits "data" and "end", and kicks off with a render call. We probably don't even need "data" because streaming templates is needlessly hard. We can change as much of this as we want to make it play nicely with other templating systems.

@larzconwell

Yeah I was thinking we were gonna have to write per engine rendering systems, Tomorrow I'm gonna plan out a way to do this efficiently!

@mde
mde commented Jun 8, 2012

Just a heads-up, our EJS implementation now passes the tests for one available via NPM (with the exception of the non-standard filters stuff). It's worth noting that there is no "official" EJS. There are two competing versions that are older than the one on NPM: http://code.google.com/p/javascript-ejs/ and http://embeddedjs.com/ The "one with the nice Web site" has historically been looked at as the de facto one.

It's also worth noting that the NPM one is missing a few basic features (%# for comments and %% for EJS literals), and is pretty Node-specific in that it assumes CommonJS require, and uses the underscore-underscore-proto-underscore-underscore hack to inherit locals on the passed-in config object. Not ideal for reuse on the client-side.

Now that ours is passing all the same tests (including the stack-trace stuff), can we all agree to use our own, and get on with our lives? :P

@larzconwell

Awesome @mde! I didn't know there wasn't a official version, I think we can just keep our custom one then if it passes the same tests as NPM version, very cool (: Also keeps down on deps which is always good!

@larzconwell

I've been looking through the current implementation, and I like how it is currently where it injects the template into the layout were the layout has the yield() function.

The only problem with that is getting other engines to recognize the functions we provide(yield, partial), and the variables, etc that users provide(the list of todo items in the examples).

@larzconwell

I've rewritten the whole template code twice, and I can't get any thing working good enough! Or working at all.

I didn't think it was going to be nearly this hard when I first started, but I think I did make it better in some ways.

It will actually run an app with other template languages, it just can't compile successfully including the yield and partial functions and anything that users include(Todo lists for example)

Jade Note: Assuming you don't have anything that is needed when compiling the files(like the yield or partial function), Jade will compile the complete layout and template¹, but will only return the layout since the way we are splitting the layout and template into two actions when compiling.

Also I made it somewhat easier to plug in if someone can get it working correctly, template/adapters/index will try to require the correct engine wrapper depending on the extension of the templates. So whenever we get this working we will need to create engine wrappers for the other languages².

All the tests pass and I've made sure everything renders correctly on the browser.
I think this PR can be merged in, so someone else can build on what I've managed to do, because I'm not sure what else I can contribute to the template engines, I've tried my hardest on it but I don't think I can do anymore with it.

Notes:
¹ With the layout but only if you use Jades native layout way, using: extends and only when include .html at the end of that
² I've created a wrapper for Jade, but I'm sure it'd need to be changed somewhat, so others could be based on that

@mde

I'll take a look at it this weekend. Thanks for your work on this. I hope we can get something working that's reasonably sane for the different systems, but also plugs in nicely to the existing structure. I know this stuff is hard. :)

@larzconwell

You're welcome, the only thing that's not working is getting the other engines to recognize the existence of our helper functions, but I couldn't find a way to get it to work because they're so closely coupled with the partial and the render functions(And have to be that way). It was extremely hard, but it was fun! I'm sorry I couldn't get it working all the way (:

@johnnygreen

Just found this on dailyjs.com - https://github.com/vdemedes/templato. It implements cross templating system helper functions. Might be something to be learned there. If I'm way off base I apologize. I haven't been following the thread very closely.

@larzconwell

Hmm that looks interesting, i'll take a look a little later today. It probably won't help much though because how coupled our helper functions are, but i'll see what I can do!

@larzconwell

Using the Templato module @johnnygreen suggested I've gotten Jade to render with the yield function and including the todo list from the examples!

http://i.imgur.com/qqb5d.png
Right now it's escaping the partial data, but I'm working on solving that (:

@mde

Awesome!

@larzconwell

@mde If this is works correctly for all the engines, should I add the module to /deps? So we don't have to get people to install it?(I'm gonna have to customize the module anyway so it uses our EJS engine, and add other engines)

@mde

Yes, it looks like it's MIT licensed, so we can stuff our customized version of it in /deps.

@larzconwell

Okay great! Thanks (:

@larzconwell

Do you guys think we should add CoffeeCup support? I'm not sure if that many people use it, it wouldn't hurt to include it though.

Also do people normally use Hogan or the Handlebars module? If it's Hogan what extension is that??
Also noticing in the Templato source, Mustache also uses the .mu extension sometimes, it doesn't mention .ms ext though.

@Techwraith
@larzconwell

Okay good idea, and I don't think I will add coffeecup. Thanks!

larzconwell added some commits Jun 16, 2012
@larzconwell larzconwell Added new Jade example
- Example may need some fixing once we get the rendering complete

Moved EJS implementation to `/deps`
- I did this so we can easily manage it ouside the template folder
- Also because it works better there with the Templato module

Include a customized version of the Templato module
- Templato will manage our helper functions
- Also will manage the actual compiling of the templates
7f414ae
@larzconwell larzconwell Updated branch 0b06664
@larzconwell larzconwell Merge branch 'extending_template_engine_support' of github.com:larzco…
…nwell/geddy into extending_template_engine_support

Conflicts:
	lib/template/adapters/ejs.js
44bc33e
@larzconwell

I included a better jade example now, and it is rendering mostly correctly, I'm gonna look through it better once I get everything finished.

Also I included the Templato module that's been customized to include mustache and our EJS implementation. and made it so the tests and the main template code uses it to register helper functions and uses it to render.

Also I moved the EJS implementation to /deps/ejs.js so that Templato can play with it easier. Now I have to re-code it so it resembles the other implementations(using a compile() and render() function), then it should be rendering correctly as well (:

After that gets done, I just need to test other languages more, create wrappers for the other languages, make it so yield isn't escaping HTML in Jade and write more tests for the other languages.

@larzconwell

@mde Hey, would you like to re-code the ejs code so it resembles the Jade source and other engines? That way it can be used with the Templato module. I would do it but I don't want it to lose any of it's current features, and you know it better since you wrote it!

Also the tests already resemble the new way, so you just have to write it then do tests as you normally would (: also if you want you can test Templato, cd deps/templato && ./test.js and it'll check to make sure they all compile and have the correct output!

@mde

I have no problem making more changes to our EJS impl. However, I would prefer it not live in deps unless we pull it out as a separate Node module. The idea with that directory is to call out third-party deps we need to distribute with our project. Can't you make the load-path configurable?

@mde

Oops, also keep in mind we'll be using this EJS impl by default in the browsers as well. We have a choice of unifying on some sort of AMD for both server- and client-side use, or doing something simple only for the modules we know we'll be using in the browser (e.g., the model stuff, EJS for starters). Not sure how we can make the templating solution pluggable on the client.

@larzconwell

Yeah I can change the paths, but I can't put it in templates/adapters/ejs.js because I have an ejs wrapper there that will mimic the current implementation(I have wrappers for all the engines like this), so where else could I put it? I could put it in templates/engines/ejs.js

Ah that'd be cool, currently does it work in the browser? I think that should be another PR, because it sounds like it'll be a bit complicated (;

@mde

Sure, templates/engines sounds good. I'll need to make at least one change to make it work in the browser again. In any case, we'll be copying it from wherever it lives in the server-code to public/js, so it can be served to the browser.

Any ideas how we'll set it up for people to use these other engines client-side? Or do we just hand-wave it away for now?

@larzconwell

I was thinking about writing some view helpers, like link_to(link, html_options), script_link(link, html_options), style_link(link, html_options) so that way it should be somewhat easier to manage links with Geddy, and maybe make it so you can link specifically to edit actions, or show actions by specifying an ID.

So if we do that we can change the generated templates, we can do something like this in the head element:

<head>
  <%= script_link('ejs_engine'); %>
  <%= script_link('ejs'); %>
  <%= script_link('geddy/ejs'); %>
</head>

Or something like that I'm thinking. then they can change whatever one we decide on to their engine, and bam it will have that engine ready (: Not sure on how we can serve those scripts though.

What do you think about that? Making view helpers doesn't seem that hard.

larzconwell added some commits Jun 19, 2012
@larzconwell larzconwell Fixed bug where helper methods were only available in layout templates,
Refactored truncate and truncateHTML utils/string functions so they match better,
and use an object for all options with nice fallbacks
d6f37e7
@larzconwell larzconwell Merge branch 'master' into extending_template_engine_support 9ab5a2c
@larzconwell larzconwell Updated Jade example so that it doesn't escape template HTML,
Wrote new Template module tests so that it tests text and HTML output for engines,
Moved ejs engine to `/lib/template/engines/ejs.js` and updated tests/templato
Finished Jade example so it matches with EJS example
6de6460
@larzconwell

Don't laugh at me, but I found out why the yield function was escaping HTML, because the engines all escape by default, so I just had to change = yield() to != yield and it doesn't escape the HTML(This usually would be a security risk, but in this case it isn't because the content returned from the function has already been escaped by the engine once), also in handlebars to return unescaped HTML you have to do {{{yield}}} instead of {{yield}} so that's done!

I also moved the EJS engine to /lib/template/engines/ejs.js and updated tests/templato
And finally I wrote a new test for Templato so it checks text and HTML.

All that needs to be done now is re-write the EJS implementation so it matches what Jade, Handlebars, etc. are using(e,g,: a compile() function) and to write Jake tests for other engines.

@Techwraith

@larzconwell That's pretty awesome. I should have chimed in a while ago about that :P

Tests would be great!

@larzconwell

Oh! and I'm currently working on a linkTo()/link_to(), scriptLink()/script_link() and a styleLink()/style_link() helper so it'll be easier to manage links to views. I already implemented truncate and truncateHTML methods that work pretty good, so I also rewrote the utils.string.truncate and utils.string.truncateHTML so they are more similar, taking the following args truncate(String, Object, Function)(TruncateHTML follows the same). Also in the /lib/template/helper.js file I am writing how to use each of the functions listed, hopefully in the future I hope to re-write it so it has it's own directory and can include most(if not all) the utils functions so it's all available in controllers and views together.

I'm planning on it to be like Rails uses it's link_to view helper(Via @Techwraith, https://twitter.com/TechWraith/status/215232695154917377), so some people will already know how to use that helper.

I'm not sure how to tackle script_link and style_link yet, I can't do Rails style because it's using Sprockets to manage assets, so I'm thinking about just return a style/script link for a specified url(Possibly not including the extension?).

@larzconwell

Oh @Techwraith you were talking about including generators for the different engines, so after this gets merged I was gonna make generators for EJS, Jade, etc.

@larzconwell

Yeah I'm not sure why I didn't catch that before, I read the docs! Just obviously not close enough!

@Techwraith

@larzconwell If you're going to tackle the generators anyways, do you want to try to tackle the new resource generators too?

Basically, we need something that will build out controller actions and views for each resource that actually do CRUD.

@larzconwell

Sounds cool! I'll look into it when I get there (:

larzconwell added some commits Jun 20, 2012
@larzconwell larzconwell Created tests for Handlebars/Mustache and Jade engines ef3a6eb
@larzconwell larzconwell Moved helpers into their own directory, and finished implementing the…
… contentTag helper

and the imageLink helper, created new string utils method called dasherize that will create a dashed string
from camelCase and snake_case string. Also create a new util called Object and made a merge method for it
the merge method merges to objects together, and supports deep merging.
e8fa586
@larzconwell larzconwell Merge branch 'master' into extending_template_engine_support 172f790
@mde

Okay, I've pushed a remote branch, extending_template_engine_support, with the EJS changes. The EJS tests all pass, but something in Handlebars breaks the test suite. What else do I need to do with this to get it in a state where it can land?

@larzconwell

I just downloaded the branch, and all the tests pass for me. For which tests are they breaking? Jake or the one in templato directory?

@larzconwell

I just had one problem with the Templato tests and that was just because /lib/geddy wasn't being loaded for EJS but once I fixed that, all tests were passing.

@mde

Running test rendering passed-in data
jake aborted.
Error: node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
TypeError: Cannot read property 'string' of undefined
at new (/Users/mde/work/geddy/lib/response/errors.js:73:32)
at [object Object]. (/Users/mde/work/geddy/lib/template/adapters/handlebars.js:31:15)
at [object Object]. (/Users/mde/work/geddy/lib/template/adapters/index.js:31:17)
at /Users/mde/work/geddy/test/handlebars_mustache.js:14:11
at Object.test rendering passed-in data (/Users/mde/work/geddy/test/handlebars_mustache.js:24:18)
at Object. (/Users/mde/work/geddy/test/handlebars_mustache.js:73:12)
at Module._compile (module.js:441:26)
at Object..js (module.js:459:10)
at Module.load (module.js:348:31)
at Function._load (module.js:308:12)
at /usr/local/lib/node_modules/jake/lib/api.js:214:18
at [object Object]. (/usr/local/lib/node_modules/jake/lib/utils/index.js:101:9)
at [object Object].emit (events.js:70:17)
at ChildProcess. (/usr/local/lib/node_modules/jake/lib/utils/index.js:267:20)
at ChildProcess.emit (events.js:70:17)
at maybeExit (child_process.js:361:16)
at Process.onexit (child_process.js:397:5)

@larzconwell

Hmmm, the only place where string is defined is in the render function and it's all correct there. Does it pass if that test is commented out?

@mde

Oh, I'm just a dumbass. I didn't have handlebars or jade installed. :) It's all fine and dandy. Do you have any docs for this stuff? How can I kick the tires on it and ensure that stuff works?

@larzconwell

Haha Nice Matthew, just install that version, then you can just start up the example you want to run! They all run now, it's completely transparent. (:

@larzconwell

I don't have docs yet, Once I finish with this urlFor helper that generates urls based on a String or Object it'll be completely done, then I'll write the main docs!

@mde

Okay, so you want the helpers to land as part of this. Sounds good. We also need a prereq Jake task to run before "test" to install any missing development deps. Failures block the entire suite, so that means people who might just want to run the model tests will trip over this.

@larzconwell

Yeah I figured it'd be good to include the helpers since I'm almost done with them.

I didn't know that, I'll add a prereq task once finished!

@mde

Would it be possible to modify the Templato stuff to return a specific error when a templating lib is missing? The best way to do this would be to set a listener for the 'error' event on the test task, and install, retry if the error is just the missing lib. We don't want to assume though that any error means you just need to install a templating lib.

@mde

Oh, and while I'm at it, I'm going to convert all the Geddy tests to the new Jake TestTask. Knocking that out right now.

@larzconwell

You want it so when Templato receives a error event it'll install the lib if the error is a missing lib then retry?

I just want to make sure I got that right!

@mde

I can do that part, if you'll just make sure that if libs aren't installed it throws an error that says so, instead of "Cannot read property 'string' of undefined."

@larzconwell

Okay thanks (:

That's the weird thing, it does throw with a message saying install blah blah blah. Just not during tests, but I fixed it now, during tests the utils aren't being loaded to Geddy so instead I'm just going to require the utils and it should be fine.

@larzconwell

By the way that wasn't actually Templato itself, it was response/errors on the viewError method. It was trying to use the utils.string.capitalize through Geddy global.

Also just to let you know when I push the next time, all the helper functions will be added to Geddy.helpers.

@mde

Ah, so you found a bug. You're fixing that one too? All the test changes are pushed now. I stubbed out in the Jakefile the place for pulling in needed libs before running tests.

@larzconwell

Yeah I fixed that bug, it was just on my branch though. Okay sounds good!

Oh by the way you can delete the main extending_template_engine_support branch if you'd like, I already jacked the EJS file out of it (:

larzconwell added some commits Jun 24, 2012
@larzconwell larzconwell Finished all the current view helpers, and finished the URL generator…
… for the helpers

Also rewrote some of the example pages so they use the view helpers

Added tests for all the helper methods, and added a default for hostname to be 'localhost'
e1221bb
@larzconwell larzconwell Merge branch 'master' into extending_template_engine_support c8d35aa
@larzconwell

Okay everything is completely done now, tomorrow I will write documentation(Where does documentation go?), get rid of Templato in deps in favor of something built directly into the template stuff now in Geddy, so it doesn't add any unnecessary dependencies(that aren't actually needed).

Then I'm going to finish what @mde was talking about above so when testing if a module isn't installed it'll install it then refire the tests.

So tomorrow if I'm not too busy I think this will be ready to merge that night!

@mde

For now, we're putting docs in the GitHub wiki pages. Longer-term I'd like to see all that stuff on the Website. Also, there's a GitHub pages site for the static API docs you can generate with JSDoc using "jake doc" into the gh-pages branch.

@Techwraith

My next project will be to migrate the docs from the wiki to the website and make it look pretty. I'll probably write some more docs for the under documented parts too.

@larzconwell

Okay, @Techwraith do just want me to put it on the wikis then let you handle the site docs? That way you can do it all at the same time.

@mde I'm not sure how I can make it check if it's a missing module and then figure out what module it was that's missing. Here's what I have currently:

task('test', function () {
  var t = jake.Task.testBase;
  // TODO: Check the type of error, npm-install the libs, and re-run
  t.addListener('error', function (e) {
    var cmd = 'npm install handlebars';
    jake.exec([cmd], function() {
      console.error('Installed module.');

      t.invoke(); // Once installed re-run the tests
    });
  });

  t.invoke();
}, {async: true});

Here's the code in the test/handlebars_mustache.js test:

try {
  require('handlebars')
} catch(err) {
  var Events = new (require('events').EventEmitter);

  Events.emit('error', err);
}

Also for some reason I'm getting this stack trace if it's just doing npm install module, but it get's installed.

geddy[extending_template_engine_support]% jake test --trace      (~/Desktop/geddy)
jake aborted.
pm" "install" "handlebars"
npm ERR! cwd /home/larz/Desktop/geddy
npm ERR! node -v v0.6.19
npm ERR! npm -v 1.1.24
npm ERR! path ../handlebars/bin/handlebars
npm ERR! code EACCES
npm ERR! message EACCES, symlink '../handlebars/bin/handlebars'
npm ERR! errno {}
npm ERR! 
npm ERR! Additional logging details can be found in:
npm ERR!     /home/larz/Desktop/geddy
pm-debug.log
npm not ok
npm not ok
@Techwraith

Yeah, just put it up in the wiki for now.

@larzconwell

Okay will do.

larzconwell added some commits Jun 24, 2012
@larzconwell larzconwell Fixed bug where apps couldn't be generated and resources couldn't be …
…created.

Bug was templates/Jakefile still referenced the old code, so it's been updated
3b29a7e
@larzconwell larzconwell If no controller is given with the `id` or `action` urlFor will attem…
…pt to get the current controller
3489b5c
@larzconwell

Just to let you guys know, I created the documentation for templating and view helpers

The only thing left to do now, is get the tests so when there's a missing engine, it'll install it and restart the tests. I tried doing it, but I'm not sure how to figure out what engine is actually missing.

@larzconwell

Well thanks man (:

@larzconwell

So I made it so it will install the missing libraries, but for some reason after it installs them and reruns the test it says:

geddy[extending_template_engine_support]% jake test --trace                                                                                           (~/Desktop/geddy)
handlebars is not installed, Jake will attempt to install it for you.
[sudo] password for larz: 
done!
jade is not installed, Jake will attempt to install it for you.
[sudo] password for larz: 
done!
jake aborted.
TypeError: Cannot call method 'addListener' of undefined
    at /usr/lib/node_modules/jake/lib/test_task.js:167:11
    at /usr/lib/node_modules/jake/lib/api.js:147:5
    at [object Object].action (/usr/lib/node_modules/jake/lib/test_task.js:87:7)
    at [object Object].run (/usr/lib/node_modules/jake/lib/task/task.js:205:21)
    at [object Object].runPrereqs (/usr/lib/node_modules/jake/lib/task/task.js:115:12)
    at [object Object].invoke (/usr/lib/node_modules/jake/lib/task/task.js:95:10)
    at [object Object].action (/usr/lib/node_modules/jake/lib/test_task.js:74:14)
    at [object Object].run (/usr/lib/node_modules/jake/lib/task/task.js:205:21)
    at [object Object].runPrereqs (/usr/lib/node_modules/jake/lib/task/task.js:115:12)
    at [object Object].invoke (/usr/lib/node_modules/jake/lib/task/task.js:95:10)

I'm not sure why as, it should be defined(Since it's getting the error event that means it's defined!).

@larzconwell

@mde Hey Matthew do you know a fix for the problem above? I'm not sure why it isn't working and not sure of a way to fix it.

Once this error gets fixed I think it'll be ready to merge.

@mde

I'll take a look ASAP.

@mde mde merged commit f20b920 into geddy:master Jun 29, 2012
@mde

I've got the test stuff sorted, and I'll be pushing it to master in just a moment.

@mde

Okay, fixed tests are pushed to master. @larzconwell, you're now a committer. Congrats. :)

@larzconwell

Thanks for getting that for me! Thanks @mde, that's incredible! :D

Sent via Hubroid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment