Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "anchored dot notation" #129

Merged
merged 1 commit into from
Aug 15, 2015
Merged

Add "anchored dot notation" #129

merged 1 commit into from
Aug 15, 2015

Conversation

bobthecow
Copy link
Owner

  1. Given that {{ . }} resolves as the top of the context stack;
  2. And when any falsey segment in a dotted name is encountered, the whole name yields '';
  3. A name like {{ .name }} should imply {{ [top of stack].name }};
  4. Thus, it must resolve as truthy only if a member of the top of the context stack matches name.

There have been several syntaxes proposed (mustache/spec#10 and mustache/spec#11 as well as my mustache/spec#52). This one is my favorite because,

  • It introduces no new symbols, it simply allows combining two existing syntaxes into a single tag.
  • It is backwards compatible, meaning, no currently working code will be broken, since {{ .foo.bar }} isn't a valid variable name in the current spec.
  • It doesn't come with any of the crazy traversal logic that the {{ ../foo }} style anchors do, so I feel it's more in keeping with the logic-free nature of Mustache.
  • It doesn't involve blessing any valid variable names as super-variables (e.g. the proposed {{ this.foo }} syntax, which would be a backwards compatibility break, as well as limiting perfectly valid variable names in some languages.

See spec discussion at mustache/spec#52 and some impetus at #98.

This is a complete implementation, but it's currently missing tests. I'm torn on including it without a pragma, since it would technically make Mustache.php not spec compliant.

Thoughts?

@rafi
Copy link

rafi commented Jan 19, 2013

So did you end up including with or without a pragma ?

@bobthecow
Copy link
Owner Author

It's in the feature branch without a pragma, but I'm leaning toward adding one before it's merged.

@rothshahar
Copy link
Contributor

@bobthecow, just curious to know, why do you think mustache was implemented so that missing data in the stack looks up data up the context stack? It sounds really ambiguous/error prone to me.
Anchored dot notation seems like a much safer/better approach and should have been the default for mustache.
Am I missing some significant benefits with the current approach? (maybe providing default values for an array of items).
Thanks.

@bobthecow
Copy link
Owner Author

The context stack is actually intuitively what you'd expect in most cases. Consider this data:

<?php

$data = array(
    'owner' => 'Justin',
    'pets'  => array(
        array('type' => 'dog', 'name' => 'Rex'),
        array('type' => 'dog', 'name' => 'Max'),
        array('type' => 'cat', 'name' => 'Sam'),
    ),
);

And this template:

<li id="pets">
  {{# pets }}
    <li>{{ name }} is a {{ type }} that belongs to {{ owner }}</li>
  {{/ pets }}
</li>

In most cases, you want to be able to reference anything in the parent context (for example, {{ owner }}) while looping through the child context. Imagine if PHP forced you to do something like this:

<?php

$owner = 'Justin';
$pets  = array(
    array('type' => 'dog', 'name' => 'Rex'),
    array('type' => 'dog', 'name' => 'Max'),
    array('type' => 'cat', 'name' => 'Sam'),
);

foreach ($pets as $pet) use ($owner) {
    echo $pet['name'] . " is a " . $pet['type'] . " that belongs to " . $owner;
}

That would be fairly absurd, especially if you needed access to more than one variable.

Now imagine if there was no way to do use... you were just stuck with whatever data was in $pet. That would be even worse, because you'd have to store all the variables you wanted from outside the loop onto each element of the array you were going to loop over.

If Mustache didn't have access to the context stack, you would run into far more problems than you currently do with this one particular edge case :)

@rothshahar
Copy link
Contributor

Thanks for the quick reply.
In the context of a single function it makes sense because you control all the variables that are in that scope.
I guess you can say that you build the context data so you can make sure that there are no collisions but -
In a larger application where a template might have several partials built by different teams, and the data is dynamically pulled from a db, it's harder to ensure that some data won't be missing from the context of one partial and accidentally be populated by data that was prepared for another partial.
I guess in this case, I'll just have to ensure that all the keys are present even if some of them have an empty string as a value.

@bobthecow
Copy link
Owner Author

@rothshahar One solution to that problem that I've used in the past is to compartmentalize the widgets as subrequests, or to run them through a manager of some sort. For example, we had a lot of CMS-y content that could be included in any template. We added a service to manage those CMS blocks, added it as a helper, and gave it a magic __isset / __get interface for querying and including the blocks dynamically:

{{{ cms.blocks.someBlockId }}}

Essentially, these were handled as subrequests, because the manager needed to be able to gather all of the data needed by a specific block, then render that block template as a standalone call, then return the rendered string to be included in the calling template.

Another possible solution would be to add syntax equivalent to jinja's {% include 'foo' with bar, baz as qux %}, i.e. a way to render a partial and only pass in explicit values to its rendering context. I do feel like this sort of thing steps across the "logic-less" line a bit too far, but it might be worth raising a suggestion like this with the @mustache crowd and seeing what people think.

@bobthecow
Copy link
Owner Author

One further possibility that I've been playing with is presented by the {{% FILTERS }} pragma... The current implementation only supports filtered interpolation, e.g. {{ foo | somefilter }}, but if it were extended to support filtered section context as well, it would be possible to create a helper that masks all context stack lookups:

It might look something like this:

<?php

class OnlyHelper
{
    private $value;

    public function __construct($value)
    {
        $this->value = $value;
    }

    public function __isset($name)
    {
        return true;
    }

    public function __get($name)
    {
        if (is_object($this->value) && !$this->value instanceof Closure) {
            if (method_exists($this->value, $name)) {
                return $this->value->$name();
            } elseif (isset($this->value->$name)) {
                return $this->value->$name;
            }
        } elseif (is_array($this->value) && array_key_exists($name, $this->value)) {
            return $this->value[$name];
        }

        return '';
    }

    public static function create($value)
    {
        if ($value instanceof Traversable) {
            $value = array_values(iterator_to_array($value));
        }

        if (self::isArray($value)) {
            return array_map(function($v) { return new OnlyHelper($v); }, $value);
        }

        return new OnlyHelper($value);
    }

    protected static function isArray($value)
    {
        if (!is_array($value)) {
            return false;
        }

        $i = 0;
        foreach ($value as $k => $v) {
            if ($k !== $i++) {
                return false;
            }
        }

        return true;
    }
}

Then it could be added as a helper:

<?php
$mustache->addHelper('only', array('OnlyHelper', 'create'));

... and used to "mask" values and prevent context stack lookups:

{{ title }}
{{# items | only }}
  {{ title }}{{! empty if current item does not have a title property or method }}
{{/ items | only }}
{{# widgetContext | only }}
  {{> widgetPartial }}
{{/ widgetContext | only }}

I'm not completely sold on the idea of filtered section contexts yet, but I figured I'd throw this idea out there as well.

@groue
Copy link

groue commented Sep 5, 2013

@bobthecow: Filtered sections are quite useful:

@smehtaCAS
Copy link

Hi @groue, @bobthecow,

I wanted to find out the decision on Filtered sections. Was it implemented or are there any plans of doing so.

@bobthecow
Copy link
Owner Author

@smehtaCAS Filtered sections are implemented in both GRMustache and Mustache.php

See https://github.com/bobthecow/mustache.php/wiki/FILTERS-pragma

@smehtaCAS
Copy link

Thanks @bobthecow. Will there be an example I can look at.

@smehtaCAS
Copy link

Actually, I was trying to do the following:

       $musObj->addHelper('utils', array(
                                        'toArray' => function($value) {
                                             return explode(',', $value);
                                        }
                            )
            );
Data:
$data = array(
    'items' => array(
                     'file_ids' => '1001,1003,1005' 
                 )
);
Template:
{{# item }}
     {{# file_ids | utils.toArray }}
         
test
{{/ file_ids }} {{/ items }}

Is this possible?

@bobthecow
Copy link
Owner Author

Yes, that will work, but this is a pragma (non-standard extension to he mustache spec) so you need to enable it at the top of your template:

{{%FILTERS}}

That said, your example should be preparing that data in code rather than using a filter in the template.

@groue
Copy link

groue commented Aug 6, 2014

That said, your example should be preparing that data in code rather than using a filter in the template

Don't be too hard, @bobthecow... data preparation is cumbersome. :-)

@smehtaCAS
Copy link

I tried the same but it does not work. It gives me nesting error. Following is what I am trying:

$m->addHelper('utils', array(
                            'toArray' => function($value) {
                                 return explode(',', $value);
                            }
                )
);
$data = array(
    'items' => array(
                    array(
                     'it_name'  => 'test1',
                     'file_ids' => '1001,1003,1005'
                    ),
                    array(
                     'it_name'  => 'test2',
                     'file_ids' => '2001,2003,2005'
                    )
                 )
);
$tem = '
{{%FILTERS}}
{{# items }}
     {{ it_name }}
     {{# file_ids | utils.toArray  }}
         test
     {{/ file_ids }}
{{/ items }}
';
echo $m->render($tem, $data);

The error is
Nesting error: file_ids | utils.toArray (on line 3) vs. file_ids (on line 5)

Any suggestion?

@bobthecow
Copy link
Owner Author

You have to put the filter in the closing tag as well.

It's ugly, I know, and that requirement might be dropped in the future. But the current Mustache spec says the opening and closing tags need to match.

@smehtaCAS
Copy link

Thats works! I had spent hours for this. Thanks!

@groue
Copy link

groue commented Aug 7, 2014

It's ugly, I know, and that requirement might be dropped in the future.

To fix this ugliness, GRMustache allows empty closing tags: {{# long section name }}...{{/}}

@bobthecow
Copy link
Owner Author

You have to put the filter in the closing tag as well.

Note that this is no longer the case. Templates like this work great:

{{%FILTERS}}
{{# states | eachPair }}
{{ key | upcase }}: {{ value }}
{{/ states }}

(See the example this came from)

1. Given that {{ . }} resolves as the top of the context stack;
2. And when any falsey segment in a dotted name is encountered, the whole name yields '';
3. A name like {{ .name }} should imply {{ [top of stack].name }};
4. Thus, it must resolve as truthy only if a member of the top of the context stack matches name.

See mustache/spec#52
bobthecow added a commit that referenced this pull request Aug 15, 2015
@bobthecow bobthecow merged commit 72b9693 into dev Aug 15, 2015
@bobthecow bobthecow deleted the feature/anchor-dot-context branch August 15, 2015 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants