Package / Naming thoughts

[ryan-mahoney]

Minor recommendations:

• Add "Handlebars" as a namespace to all classes
• Enable true autoloading via composer
• Remove underscores from class names
• Consider changing Handlebars_Engine to simply Handlebars

Then to use the class, we would do:

use Handlebars\Handlebars;

Handlebars->template($template, $data);

[ryan-mahoney]

I have made these changes in my fork and have done some cursory testing. Please let me know what you think. I know it's not really a big deal... but I think it makes the code more "modern" feeling.

Also, the current composer.json file makes it seems as if there is a namespace, when in fact in the current code there is not.

You can see this in my fork: https://github.com/virtuecenter/handlebars.php

Thanks, Ryan

[everplays]

I'll keep this issue open (if you don't mind), until I find some free time to:

1. make another branch for php 5.2.x then I can merge your branch as your changes will make the library 5.3+
2. write a proper readme
3. write a few tests for library.

[ryan-mahoney]

Sound good, thanks so much

[miccheng]

+1. Ditch 5.2.x already.

[everplays]

for the record:

• now we've namespace support.
• codes pass PHP CodeSniffer.
• we've better readme file (yet not complete).

Hopefully, I'll start writing tests in a few days.

[boukeversteegh]

I had trouble importing Handlebars after the introduction of the namespace. I think I don't understand well how namespaces work, but this is how I eventually got it to work inside my CakePHP application. Specifically, I did not write use Handlebars\Handlebars.

class PageController extends AppController {
    public function show($id) {
        require_once current(App::path('Vendor')) . '/Handlebars/Autoloader.php';
        Handlebars\Autoloader::register();
        $hdlbars = new Handlebars\Handlebars();

        ...
        $hdlbars->render($template, $recordData);
    }

}
....

Whenever I tried to use the namespace, I got errors (class Autoloader not found) or warnings (The use statement with non-compound name 'Handlebars' has no effect).

[everplays]

@boukeversteegh if you're using use keyword for loading Autoloader, that's normal (there's no autoloader yet to autoload it) unless you use composer and in that case, you don't need to use Autoloader at all, composer takes care of autoloading. if you've something like this

$h = new Handlebars\Handlebars;

you can replace it with

use Handlebars\Handlebars;

$h = new Handlebars

which is useful when you make lots of instances.

[everplays]

thanks to @fzerorubigd, now we've tests setup. still we need to add more test, but I'm gonna close this issue.

[ryan-mahoney]

@everplays great work. Looking forward to checking this out!