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
feat: Use PropertyAccess Component syntax for nested variables access #302
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,4 @@ | |
/my-app.* | ||
/var/ | ||
/vendor/ | ||
.idea/ | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
|
||
namespace Castor; | ||
|
||
use Symfony\Component\PropertyAccess\PropertyAccess; | ||
|
||
/** @internal */ | ||
class ContextRegistry | ||
{ | ||
|
@@ -113,11 +115,11 @@ public function getVariable(string $key, mixed $default = null): mixed | |
{ | ||
$context = $this->getCurrentContext(); | ||
|
||
if (!isset($context[$key])) { | ||
return $default; | ||
if (str_starts_with($key, '[')) { | ||
return PropertyAccess::createPropertyAccessor()->getValue($context, $key) ?? $default; | ||
Comment on lines
+118
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed this is a bit weird. Could we find a way to always use property access? As a user, I would expect:
But When I write this, I really wonder why it's better than $a = variable('nested')['a'] (Note: I'm not against this PR. Just asking) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but the component syntax doesn't allow you to do that, as far as I know. An element of an array must be enclosed in square brackets like The only thing that's better than syntax two (current), is that the default value must be managed again in the case, for example, that the index doesn't exist.
And worse if
But as indicated in the comment here #302 (comment), this approach could be handled in pure PHP without any deps |
||
} | ||
|
||
return $context[$key]; | ||
return $context[$key] ?? $default; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: dynamic | |
Production? no | ||
verbosity: 1 | ||
context: baz | ||
nested key: N/A |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: my_default | |
Production? no | ||
verbosity: 2 | ||
context: bar | ||
nested key: nested_value |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: path | |
Production? yes | ||
verbosity: 1 | ||
context: bar | ||
nested key: N/A |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: production | |
Production? yes | ||
verbosity: 1 | ||
context: bar | ||
nested key: nested_value |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: run | |
Production? no | ||
verbosity: 1 | ||
context: no defined | ||
nested key: N/A |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ context name: my_default | |
Production? no | ||
verbosity: 1 | ||
context: bar | ||
nested key: nested_value |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,5 @@ context name: dynamic | |
Production? no | ||
verbosity: -1 | ||
context: bar | ||
nested key: N/A | ||
bar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you revert that? It must be in your global git ignore
https://stackoverflow.com/a/7335487/685587