Make template escape methods public about plates HOT 20 CLOSED

This has been fixed! See 3688ad5.

from plates.

+1 on this issue. I have an extension that I want to escape input on, but currently can't without duplication of the escape method.

from plates.

Hey gents, I'm a little behind on my Plates tickets due to some recent focus on my other library. However, I'll have a look at this closer in a couple days. Thanks for your patience!

from plates.

@reinink A PR for this change should be relatively short and simple. If you want me to submit one, just poke me while we're both at Lone Star PHP.

from plates.

+1 for this. Like @jelofson, I have extensions that need to do escaping, and it would be much better to rely on the built-in escape method rather than duplicating it.

from plates.

Sorry for the long delay here folks. I've started a PR for this: #122

My only real concern at this point is what to tag this "fix" as. I'm thinking it could be considered a simple bug fix, however, if anyone has created extensions using these function names, they will stop working and it will use the defaults instead. However, doing a 4.0 release for this seems much too extreme as well.

Any thoughts?

from plates.

@reinink

if anyone has created extensions using these function names, they will stop working and it will use the defaults instead

Not sure I follow what you mean. Can you provide a code example that shows this behavior?

from plates.

Basically I just mean that if someone created an extension using one of the default functions (ie. escape), then this patch will now switch back to using the Plates function, not the extension function. For example:

class MyEscapeExtension implements \League\Plates\Extension\ExtensionInterface
{
    public $template;

    public function register(\League\Plates\Engine $engine)
    {
        $engine->registerFunction('escape', [$this, 'escape']);
    }

    public function escape()
    {
        // some escape functionality
    }
}

However, maybe this isn't an issue. If someone replaces a default Plates function with their own extension, then maybe a break caused by this "fix" is their own fault?

from plates.

@reinink

However, maybe this isn't an issue. If someone replaces a default Plates function with their own extension, then maybe a break caused by this "fix" is their own fault?

I wouldn't put the onus on the user to predict potential future core method names. More than likely, they've added their own functionality that core was lacking at the time, and that sort of innovation should be encouraged.

That said, if there's an easy way to adjust their code to avoid changing its behavior as a result of upgrading to a version with this "fix," that would be preferable, and may not necessarily necessitate a major version bump.

I'm not sure how extension method names are resolved as compared to core method names. Does it have to do with the order in which extensions are loaded? Do extension methods simply take precedence over core methods? Does core lack a way to resolve such conflicts, and maybe a method of doing so needs to be introduced?

from plates.

I wouldn't put the onus on the user to predict potential future core method names.

I totally agree. I'm talking about existing core method names.

Does it have to do with the order in which extensions are loaded?

The core methods take priority, and then extension functions are called if those functions do not already exist.

from plates.

I totally agree. I'm talking about existing core method names.

Clarification: existing core method names that, as in this case, would end up taking precedence over existing extension method names in the event of a conflict.

The core methods take priority, and then extension functions are called if those functions do not already exist.

Would it be possible to add a method of explicitly overriding core methods with extension methods? I ask that question with some skepticism, as it seems like this change would mean giving users enough rope to hang themselves with. However, it seems worth consideration nonetheless, as it would provide a way for users with conflicts in this scenario to retain any existing behavior of theirs despite the effects of the "fix."

from plates.

I think the concern is valid if someone did write their own escape extension. That was something I hadn't even considered. All I want to do is the following:

class MyExtension implements \League\Plates\Extension\ExtensionInterface
{
    public $template;

    public function register(\League\Plates\Engine $engine)
    {
        $engine->registerFunction('makeAwesome', [$this, 'awesome']);
    }

    public function awesome($string)
    {
        // Do some stuff with string
        $newString = str_rot13($string);
        // my string needs some escaping so let's leverage the $template's escape method
        // this only works if the Template::escape() method is public.
       return $this->template->escape($newString);
    }
}

I did not think you could even write a custom escaping extension with the name 'escape' that would override the template's escape method. Am I wrong?

from plates.

Now that I think about it, the example above by @reinink for a custom escaper with the registered name 'escape' would not work because extensions rely on __call(). You could register an extension called 'escape', but it would not ever work inside a template because that method exists in the Template class.

Correct? Maybe I should dig in there a bit before I go spouting off something that might be wrong :)

from plates.

I did not think you could even write a custom escaping extension with the name 'escape' that would override the template's escape method. Am I wrong?

With the current design, if you write a custom escaping extension, it would override the default function. However, with this proposed change (#122), it would not override the default function.

So while it would maybe we odd for someone to have done this, it is possible, and it's possible that this switch could break their setup.

from plates.

With the current design, if you write a custom escaping extension, it would override the default function. However, with this proposed change (#122), it would not override the default function.

Interesting. I just tried the following ($view is an Engine object):

$view->registerFunction('escape', function ($string) {
    return 'your string is ' . $string;
});

And in my template I called $this->escape('whatever'); and the default function is not overridden.

<h1>blah</h1>
<?=$this->escape('some string'); ?>

I also wrote a custom extension with 'escape' as the name and registered it. Again, the default escape method is not overridden. Maybe I don't understand how it can be overridden when calling registered function relies on __call().

Perhaps I am doing something differently. No bid deal. Whatever you decide is fine with me.

from plates.

Yeah maybe I better do some additional testing here first. I want to make sure I know what the side affects of this change are. Leave it with me!

from plates.

You know, another possible solution is to move escape and batch out of the Template class and make it an extension. This might simplify everything because these funcs really acts more like a plugin than a core feature related to the template anyways.

This would also resolve #96 because batch would be able to see the escape function.

from plates.

@ragboyjr

This would also resolve #92 because batch would be able to see the escape function.

Think you meant #39 or #40 instead of #92 ?

from plates.

@elazar I think i meant #96, sorry :p.

from plates.

Thanks @reinink! :)

from plates.