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

Use PropertyAccess for extend usability of getFieldValue #2262

Closed
wants to merge 1 commit into from

Conversation

@webda2l
Copy link

webda2l commented Jul 28, 2014

Current implementation is a bit too light (getter/isser) when PropertyAccess already used elsewhere in this bundle offer more (hasser/magic function)
This allow bundles like A2lixI18nDoctrine Bundle and KnpLabsDoctrineBehaviors which allow a __call method as proxy method to work with ease with Sonata.

See http://a2lix.fr/blog/2014/08/04/translate-internationalize-doctrine-entities-sonata.html

Any thoughts?

@webda2l webda2l changed the title [WIP] Use PropertyAccess for extend usability of getFieldValue Use PropertyAccess for extend usability of getFieldValue Jul 29, 2014

throw new NoValueException(sprintf('Unable to retrieve the value of `%s`', $this->getName()));
try {
return $accessor->getValue($object, $fieldName);

This comment has been minimized.

Copy link
@rande

rande Aug 11, 2014

Member

is the Accessor can pass the $paramters options ? if not it is a BC break

This comment has been minimized.

Copy link
@webda2l

webda2l Aug 12, 2014

Author

No it can't. https://github.com/symfony/PropertyAccess/blob/master/PropertyAccessor.php#L52
But with comment "// parameters for the method given in the code option" https://github.com/sonata-project/SonataAdminBundle/pull/2262/files#diff-67a6ff31e9d77cbbe3c75d3e24734453L337, I supposed that was only used with code option and so, do not manage this case with PropertyAccess.

@webda2l
Copy link
Author

webda2l commented Sep 1, 2014

For symfony2.6 and upper, use symfony/symfony#11731 is an another possibility.

@core23
Copy link
Member

core23 commented Apr 15, 2016

Are you still working on this? Please close or rebase this PR @webda2l

@core23 core23 closed this Apr 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.