Skip to content

[PropertyAccess] Document usage of createPropertyAccessor args #6640

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

Closed

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jun 6, 2016

This documents symfony/symfony#18977 (on hold).
Suggestions to make it better would be really appreciated.

@chalasr
Copy link
Member Author

chalasr commented Jun 6, 2016

Not sure that the failed test on travis is related.

.. caution::

As you can see, the method will return ``null`` if the index does not exists.
To make it throws an exception in case of invalid index, set the first argument
Copy link
Contributor

@ogizanagi ogizanagi Jun 6, 2016

Choose a reason for hiding this comment

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

-To make it throws
+ To make it throw

(present infinitive)

@chalasr chalasr force-pushed the property_access_throw_on_invalid_index branch 2 times, most recently from eae275c to 5dfc8b2 Compare June 6, 2016 22:19
As you can see, the method will return ``null`` if the index does not exists.
To make it throw an exception in case of invalid index, set the first argument
:method:`PropertyAccess::createPropertyAccessor<Symfony\\Component\\PropertyAccess\\PropertyAccess::createPropertyAccessor()>`
to true, e.g. ``PropertyAccess::createPropertyAccessor(true)``.
Copy link
Contributor

Choose a reason for hiding this comment

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

true in ticks? not sure

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean that I should wrap it in ```` chars? It's a mistake that I'll correct, thank's for catch it.

Copy link
Contributor

Choose a reason for hiding this comment

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

thats what i meant, thank you!

@chalasr chalasr force-pushed the property_access_throw_on_invalid_index branch from a64bbc2 to f65ad3a Compare June 7, 2016 10:45
@xabbuh xabbuh added the On hold label Jun 8, 2016
fabpot added a commit to symfony/symfony that referenced this pull request Jun 9, 2016
…ss::createPropertyAccessor() (chalasr)

This PR was squashed before being merged into the 3.2-dev branch (closes #18977).

Discussion
----------

[PropertyAccess] Add missing arguments to PropertyAccess::createPropertyAccessor()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | symfony/symfony-docs#6640

Actually, the recommended way to use the PropertyAccessor is to use `PropertyAccess::createPropertyAccessor`.

The problem is that using this way, we can't specify that invalid indexes should throw an exception, and so when calling `PropertyAccessor::isReadable([], '[foo]')` it returns always true.

It should be possible to make the exception thrown, plus the `PropertyAccessor::$throwExceptionOnInvalidIndex` already exists but is not used in `PropertyAccess::createPropertyAccessor`.

Commits
-------

5ded804 [PropertyAccess] Add missing arguments to PropertyAccess::createPropertyAccessor()
symfony-splitter pushed a commit to symfony/property-access that referenced this pull request Jun 9, 2016
…ss::createPropertyAccessor() (chalasr)

This PR was squashed before being merged into the 3.2-dev branch (closes #18977).

Discussion
----------

[PropertyAccess] Add missing arguments to PropertyAccess::createPropertyAccessor()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | symfony/symfony-docs#6640

Actually, the recommended way to use the PropertyAccessor is to use `PropertyAccess::createPropertyAccessor`.

The problem is that using this way, we can't specify that invalid indexes should throw an exception, and so when calling `PropertyAccessor::isReadable([], '[foo]')` it returns always true.

It should be possible to make the exception thrown, plus the `PropertyAccessor::$throwExceptionOnInvalidIndex` already exists but is not used in `PropertyAccess::createPropertyAccessor`.

Commits
-------

5ded804 [PropertyAccess] Add missing arguments to PropertyAccess::createPropertyAccessor()
@chalasr
Copy link
Member Author

chalasr commented Jun 9, 2016

The related PR has been merged (symfony/symfony#18977).
It also introduced a second argument corresponding to PropertyAccessorBuilder::$magicCall, let me know if you think that I should also adapt the documented usage of this argument (actually documented through PropertyAccessorBuilder::enableMagicCall, not directly from PropertyAccess::createPropertyAccessor() that can now be used).

@chalasr chalasr force-pushed the property_access_throw_on_invalid_index branch from 33357dd to 16738a5 Compare June 13, 2016 11:59
@chalasr chalasr changed the title [PropertyAccess] Document $throwArgumentOnInvalidIndex arg for createPropertyAccessor [PropertyAccess] Document usage of createPropertyAccessor args Jun 13, 2016
@chalasr
Copy link
Member Author

chalasr commented Jun 13, 2016

Updated according to symfony/symfony#19040.
Please don't merge it before.

Fix spelling typos

Formatting/Spelling fixes

Document features enabling from createPropertyAccessor()

wording
@chalasr chalasr force-pushed the property_access_throw_on_invalid_index branch from 16738a5 to 34bc5a6 Compare June 16, 2016 10:20
@fabpot
Copy link
Member

fabpot commented Jun 21, 2016

Closing as symfony/symfony#18977 has been reverted.

@fabpot fabpot closed this Jun 21, 2016
@chalasr chalasr deleted the property_access_throw_on_invalid_index branch August 16, 2016 12:01
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