Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Scope objectKey in nested partialLoop call #7093

Closed
wants to merge 5 commits into from

Conversation

larsnystrom
Copy link
Contributor

This is a fix for #3758 and introduces an objectKeyStack which scopes the objectKey to a nestingLevel. This allows nested calls with the PartialLoop View Helper where each level of nesting uses it's own or a parent's objectKey.

It is not clear whether this is a bug fix or a feature request which introduces a BC break, but at least no tests are failing. I'd be happy to write a test case for this fix if necessary.

This is my first pull request ever, so please be gentle when providing feedback :)

@Ocramius
Copy link
Member

Ocramius commented Jan 2, 2015

@larsnystrom can you please provide a test-case that verifies what is explained in #3758?

*
* @param string $key
*/
public function setObjectKey($key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That method overrides setObjectKey() in Partial, which we inherit. Maybe I should've added that to the docblock, but I'm unsure of the syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use {@inheritDoc}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I can use that tag for other things than classes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use that for anything that is API overriding/implementing ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess my documentation skills needs to be improved then :) Pushed a fix for it now.

@larsnystrom
Copy link
Contributor Author

I added a test for issue #3758. My commit should make it pass.

@@ -0,0 +1,10 @@
<?php
$vars = $this->vars();
if (empty($vars)) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, it ignored in .php_cs config ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh well, I've already pushed a fix :)

@Ocramius Ocramius added this to the 2.3.4 milestone Jan 2, 2015
@Ocramius Ocramius self-assigned this Jan 2, 2015
*
* @return self
*/
protected function nestObjectKey()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make these new methods private upon merge, since they really are a deep implementation detail

Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
@Ocramius Ocramius closed this in 6bf0b23 Jan 3, 2015
Ocramius added a commit that referenced this pull request Jan 3, 2015
…op-calls' into develop

Close #7093
Close #3758
Forward port #7093
Forward port #3758
@Ocramius
Copy link
Member

Ocramius commented Jan 3, 2015

@larsnystrom merged, thanks!

master: 6bf0b23
develop: 36b3714

gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…aking scope emulation variables `private`
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…ixed return type (`self`), leveraging parent class API
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…efactoring `PartialLoop` `$values` extraction logic into its own private method
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…dding `@group` annotation for newly introduced tests
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…dding test for non-iterable data exception message
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…etter exception message in case of non-iterable values given to the PartialLop helper
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…dding test for non-iterable object data exception message
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants