Talesoft / tale-jade

A complete and fully-functional implementation of the Jade template language for PHP
http://jade.talesoft.codes
MIT License
88 stars 10 forks source link

Fix quote-escaping in escaped text and text-blocks #57

Open elquimista opened 8 years ago

elquimista commented 8 years ago

Hi Torben, thanks for your hard working on this tale-jade project. I'd like a new feature to be added if it's possible for you. Here's a breakdown:

p= $faker->name
p!= $faker->name

In the above code, both cases do isset() checking before echoing the value $faker->name. I know it's used to make sure safety and convenience. But the fact is that $faker->name is not a real public member variable, it's a magic property. So isset($faker->name) returns false, thus echoing empty value ''. So I think there should be a way to resolve this case. Maybe we can use !!= symbol for this?

TorbenKoehn commented 8 years ago

You can implement isset()-logic with the magic method __isset() inside the class.

<?php

class DataObject
{

    private $_data;

    public function __construct(array $data)
    {

        $this->_data = $data;
    }

    public function __get($key) 
    { 
        return $this->_data[$key]; 
    }

    public function __isset($key) 
    { 

        return isset($this->_data[$key]); 
    }
}

$do = new DataObject(['name' => 'Torben']);

var_dump(isset($do->name)); //true
var_dump(isset($do->notName)); //false

This makes sense on most classes that also use __get.

Even though, I think another escape operator meaning "Give this value through 1:1" would be a neat thing. It would try to not put any additional code around the expressions.

Two characters (!!) is a bit harder for the lexer, so I'd suggest another escape character next to !.

What do you think about p?= $faker->name ?

It would be easy to write for everyone and also has some meaning "this expression might not exist/throw an error"

Another possible one would be p@= $faker-name

elquimista commented 8 years ago

Here, $faker variable is an instance of \Faker\Generator class, which is created by calling \Faker\Factory::create(). You can refer to this class at https://github.com/fzaninotto/Faker, so I am not able to patch the class. ?= and @=, anything is possible. We can also use ==; this double equal symbol is used in slim language as an equivalent of != in jade. I'll leave selection to you.

TorbenKoehn commented 8 years ago

I use ? now. It's called the "Unchecked"-Operator. You can combine it with the !-escaping-operator

Examples: p= $var uses isset p?= $var doesn't use isset p!= $var uses isset, doesn't escape p?!= $var doesn't use isset and doesn't escape

It works with Expressions (p?= $var), Attributes (a(href?=$var)) and interpolation (?#{$var})

It will be in the next release.

elquimista commented 8 years ago

Thanks. I'll be waiting for the next release.

And I've got another issue. Consider the following:

pre: code.
    <?php
    $foo = 'hey';
   ...

I expected the < symbol would be escaped so that it would be properly displayed in html output. But in fact, it's not being escaped so php syntax error comes out. What is the correct way to tell the tale jade parser to escape such string?

TorbenKoehn commented 8 years ago

Right now you can only use an expression or code for this (echo or <?=)

e.g.

pre: code= '<?php $foo = \'hey\';'

This is actually pretty interesting. JavaScript doesn't have such problem as JavaScript isn't parsed like PHP inside HTML (or any kind of document) Maybe plain-text should be escapable as well. Right now it is passed through directly (Which is the official behaviour).

I'm just thinking about an elegant way to solve this.

We have text in the following forms:

p.
    multiline
    text

| Single line text

p Element single-line text

:filter
    multine
    text

What I am thinking about is some kind of escape-operator, probably ! since it aligns well with escape/don't escape in attributes and expressions.

Example:

p!.
    multiline
    text

!| Single line text

p! Element single-line text

!:filter
    multine
    text

What do you think?

elquimista commented 8 years ago

That would be great!

elquimista commented 8 years ago

So I hope both the new interpolation operator (?) and non-code text escaping operators (!, !|, !.) to be included in the next release. Btw, I thought about !:filter operator and I don't think it makes sense. All text under :filter are mostly filter-specific code (css, javascript, php, markdown, coffeescript, sass, less etc), so they should be treated as raw text and shouldn't be escaped. What do you think?

TorbenKoehn commented 8 years ago

It's handled as raw text by default.

All texts are passed through completely raw, the ! in this case wouldn't mean don't escape like in expressions, it would rather mean escape :)

But you might be right anyways, if you need escaping in filters, you just add a new filter.

That's also a way you can handle your case.

Add a new filter named code

<?php

use Tale\Jade\Filter;

$renderer->addFilter('code', function($node, $indent, $newLine) {

    return htmlentities(Filter::filterPlain($node, $indent, $newLine), \ENT_QUOTES, 'UTF-8');
});

Use it

pre: code: :code
    <?php
        $someStuff = 14;
    ?>

(Didn't test that, but it should work haha)

TorbenKoehn commented 8 years ago

Maybe the filter-method is even better than adding a completely new operator for this. Can you test it and see, if it gives the desired result and works neatly?

elquimista commented 8 years ago

Then I have to modify cakephp-jade plugin to add that new filter. But what if we need to escape single-line text? Rewriting single line to multiline just for escaping doesn't sound good. So I think we need these operators -- !, !|, !., to use as follows:

p! Hello <world>!
p
    !| HI <there>
p!.
    I'm <brackets>
elquimista commented 8 years ago

Mmm.. I think this symbol selection (!) might arouse confusion among users. In code block, it's used for non-escaping (!=), whereas in text block, it's going to be used for escaping. If we're going to adopt this new operator, we have to think carefully about choosing the right symbol.

TorbenKoehn commented 8 years ago

It's more like "Without it, default behaviour is used everywhere, with it, change the default behaviour"

I can't think of another character that would fit in that well.

I'm open for suggestions :)

elquimista commented 8 years ago

Ok. Let's go with that symbol then. Can't wait to see the next release. ;)

TorbenKoehn commented 8 years ago

? and text-based-! have been added.

Test them and tell me if I can close this then please :)

https://github.com/Talesoft/tale-jade/releases/tag/1.4.0

elquimista commented 8 years ago

Looks like ? operator works fine. But I have issues with ! operator.

pre: code!.
    <?php
    ...
    public $components = ['RequestHandler', 'Flash', 'Auth'];

The above code generates the following, which causes an php syntax error:

<?=htmlentities('public $components = ['RequestHandler', 'Flash', 'Auth'];', \ENT_QUOTES, 'UTF-8')?>

(Look at the single quotes)

TorbenKoehn commented 8 years ago

Jup, I need escaping there.

Nice find :) I will fix it. In the mean-time you might fix this by adding \ in front of each ', but you'll have to remove them later

elquimista commented 8 years ago

I'll wait for the fix.

TorbenKoehn commented 8 years ago

Please use \ for now, I made it keeping escaped quotes if they are already there.

The fix will take some time, since I will unify some of the expression/text/escaping/checking a bit so that errors like this can easily be avoided in the future.

After the fix your code can stay as it is and will work correctly, but new code doesn't require the \ anymore.

elquimista commented 8 years ago

Thanks :)