MyBB Community Forums

Right now you can't deny the current method of rendering templates is... ugly. And way too easy to mess up.

​eval("\$addevent = \"".$templates->get("calendar_addevent")."\";");

What if there were a cleaner, easier to use way?

​eval(render_template('addevent', "calendar_addevent"));

Adding such a function would help clean things up and make script-breaking typos more difficult. Here's mine:

​function render_template($var, $temp, $append = false)
{
	global $templates;
	return '$' . $var . ($append? ' .= "' : ' = "') . $templates->get($temp) . '";';
}

The hard part would be updating the entire codebase to use this function...

Edit: Actually it might make more sense to add it to the Templates class, a la this:

eval($templates->render($template, $variable, $append));

Unfortunately due to the way the whole thing works it still has to be wrapped in the eval() rather than moving it in to the function, but it's a good step in the right direction.

No idea why you're using double quotes to make it even more ugly (yes, I know MyBB has it in many many places too). Apostrophes feel lonely:

​eval('$addevent = "'.$templates->get('calendar_addevent').'";');

3 useless characters less and it looks much cleaner for me. That and a good PHP editor and typos are unlikely.

I directly copied that from the MyBB source. Even still, the version you posted is not pretty and kind of easy to forget a character. Especially that string bit at the end with the mish-mash of characters ').'";');

I've already gotten the method written and ready to commit:

	/**
	 * Prepare a template for rendering to a variable/
	 *
	 * @param string The name of the template to get.
	 * @param string The name of the variable to render to.
	 * @param boolean True if template is to be appended to end of variable, false if it is to replace it.
	 */
	function render($template, $variable, $append=false)
	{
		$assignment = '="';
		
		if($append)
		{
			$assignment = '.="';
		}
		
		return '$'.$variable.$assignment.$this->get($template).'";';
	}

I removed the ternary operator (even though I'd prefer it) because of coding standards. Beyond that it's more or less exactly the function I currently use. It makes using templates stupid easy.

I like it but I doubt that we'll include it as we would need to update every file to use the function. (A function which is never used is useless).

I'd be willing to go over that. Should I do it all in one PR or spread it out over several? I'm asking because doing this will touch a LOT of files...

Edit: Done updating files, ready and waiting to start commit(s)... preliminary testing shows everything is okay.

Most of the time was spent bashing my face against the keyboard until the right sed script formed.

Feel free to create one PR - just reference this thread as "issue", I'll take care of it over the weekend and try to merge it.

Shouldn't it be a function of the $templates class, since all it does is call $templates-> anyway?

Note that $templates->get() has some extra parameters too (disable HTML comments and such).

Done. I'm still flailing about with git so if I did something horribly wrong let me know so I can redo it.

https://github.com/mybb/mybb/pull/1292

@frostschutz: He did both - added it to class_templates and added the two parameters.

Pull Request 2.0: Now with less fail!

https://github.com/mybb/mybb/pull/1293