Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Array rendering #78

Closed
mcharytoniuk opened this Issue · 11 comments

4 participants

@mcharytoniuk

I think there is a bug in Mustache.php code at line:

while ($next = array_shift($chunks)) {

When passing array like "array('foo', 'bar');" into the template like: "{{array.0}} {{array.1}}", the output is "Array bar" instead of expected "foo bar", because "($next = array_shift($chunks))" at key "0" evaluates to false and full array is returned.

My proposed solution is:

while (count($chunks)) {
    // Slice off a chunk of context for dot notation traversal.
    $next = array_shift($chunks);
@bobthecow
Owner

You know, you're absolutely right. I've never used dot notation with numeric indices, so I hadn't noticed.

I would happily accept a pull request with a test case and a this fix if you'd like to submit one. Otherwise I'll fix it in the next release.

Thanks!

@mcharytoniuk

OK. Expect my test case. :)

@Prinzhorn

Since I'm a performance fetishist, I'd go with:

while (($next = array_shift($chunks)) !== NULL) {

just saying ;-)

@bobthecow
Owner

@Prinzhorn Except that fails with

$foo = array('a', null, 'b');

... Which is a weird thing to iterate over, but it still should work.

@Prinzhorn

Wow, you're right.
I just feel uncomfortable with "count", because we don't need to know the number of entries. I don't do a lot of php, but would this work then?

//Before
while (count($chunks)) {

//After
while (!empty($chunks)) {
@mcharytoniuk

I think 'empty' is better, thanks.

@Prinzhorn

You're welcome.

@bobthecow
Owner

Agreed. Let's go with !empty

@scribu

Why not just use foreach ( $chunks as $next ) ? It would definitely be faster than while.

@bobthecow
Owner

@scribu Good call.

@bobthecow
Owner

@jwronsky I pulled your changes from #80 but updated it to use foreach as @scribu recommended. Thanks!

@bobthecow bobthecow closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.