[WIP] removal of duplicate about attribute in collections rendering

[adou600]

This PR try to address #29 and #44.

The idea is to add a noautotag tag in twig to allow the user not to render the enclosing html element automatically. By default, this tag is not set and the enclosing div is rendered with the rdf attributes. To avoid the about attribute duplication, Collection::renderAttributeschecks if the parent is currently rendering and if yes removes the about attribute.

Do not merge this PR yet, some changes are still to come:

• new test without noautotag in the twig template
• new test to detect the case where an about property is removed
• README update with noautotag information

[adou600]

CC @dbu

[dbu]

can you please call this $attributesToSkip (they are not removed, just not printed) and add a doc comment about it?

[dbu]

actually the doc comment should go to the NodeInterface method.

[adou600]

@dbu @flack I think this PR is ready for the review.

The README is updated and the tests have been refactored to allow to test the twig extensions with and without noautotag (CreatephpExtensionTest::testBasicTemplates). A complete example for collections rendering in twig is also available (CreatephpExtensionTest::testCollectionsTemplate).

[dbu]

i think the first example should be using the autotag feature, and then we add a section how to have more control over the html from the template where we explain the noautotag

[adou600]

Actually if you look at https://github.com/symfony-cmf/createphp/blob/d146e4d71dd0797ffc0c4ccc859c4cf9a8e2ab1a/README.md#twig-extension, the first example is using the autotag:
{% createphp mymodel %}{{ mymodel_rdf|raw }}{% endcreatephp %}
Is it ok like this?

[dbu]

ah sorry, just saw that this replaced something existing and thought there is no more example without noautotag. all fine then.

[dbu]

@flack: i put some explanatory comments into the code for you. the bigger number of changes is in that adrien uses the real metadata loader in the tests now instead of mocking, which makes the tests more reliable.

@adou600 maybe you could move some of the comments i made in the PR as comments into the code so that people reading the code later see the reasoning too.

[dbu]

this looks like a conflict with #47

[lsmith77]

well it just makes #47 superfluous .. so if this PR gets merged that PR can get closed. i think its ok to include it here too.

[lsmith77]

btw just FYI for the future you could also just cherry pick my commit and then the other PR would be automatically closed and no conflict could occur.

[adou600]

Thanks for the hint! I had to change it after the dependencies update on the sandbox. As we don't know how easily #46 will be merged, may be we should keep #47 ?

[lsmith77]

the { should be on the next line

[flack]

How would you feel about using a typehint here? i.e.

public function renderAttributes(array $attributesToSkip = array());

just to make it a bit more robust

[dbu]

willdo

[dbu]

i just pushed changes according to your comments above. good to merge?

[flack]

Well, right now the tests fail accoring to Travis: https://travis-ci.org/flack/createphp/builds/4791902

Other than that, I think we're good :)

[dbu]

good thing we have the tests. i did not think very far in my last commit. now it should be good, at least it was locally.