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

feat: Use PropertyAccess Component syntax for nested variables access #302

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -5,3 +5,4 @@
/my-app.*
/var/
/vendor/
.idea/
Copy link
Member

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

1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
* Add a `yaml_dump()` function to dump any PHP value to a YAML string
* Add a `yaml_parse()` function to parse a YAML string to a PHP value
* Remove the default timeout of 60 seconds from the Context
* Add possibility to use [Property Access Component](https://symfony.com/doc/current/components/property_access.html) syntax for getting variable from context.

## 0.13.1 (2024-02-27)

Expand Down
1 change: 1 addition & 0 deletions composer.json
Expand Up @@ -43,6 +43,7 @@
"symfony/http-client": "^6.4.3",
"symfony/monolog-bridge": "^6.4.3",
"symfony/process": "^6.4.3",
"symfony/property-access": "^6.4",
"symfony/string": "^6.4.3",
"symfony/translation-contracts": "^3.4.1",
"symfony/var-dumper": "^6.4.3",
Expand Down
164 changes: 162 additions & 2 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 8 additions & 6 deletions doc/getting-started/context.md
Expand Up @@ -53,13 +53,15 @@ function foo(): void
{
$foobar = variable('foobar', 'default value');

// Same as:
$context = context();
try {
$foobar = $context['foobar'];
} catch (\OutOfBoundsException) {
$foobar = 'default value;
}
// Same as:
$foobar = $context['foobar'] ?? 'default value';

// Getting nested values
$foobar = variable('[foo][bar][baz]', 'default value');

// Same as:
$foobar = $context['foo']['bar']['baz'] ?? 'default value';
}
```

Expand Down
4 changes: 4 additions & 0 deletions examples/context.php
Expand Up @@ -21,6 +21,9 @@ function defaultContext(): Context
'name' => 'my_default',
'production' => false,
'foo' => 'bar',
'nested' => [
'key' => 'nested_value',
],
]);
}

Expand Down Expand Up @@ -83,6 +86,7 @@ function contextInfo(): void
echo 'Production? ' . (variable('production', false) ? 'yes' : 'no') . "\n";
echo "verbosity: {$context->verbosityLevel->value}\n";
echo 'context: ' . variable('foo', 'N/A') . "\n";
echo 'nested key: ' . variable('[nested][key]', 'N/A') . "\n";
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Attribute/AsCommandArgument.php
Expand Up @@ -5,7 +5,7 @@
abstract class AsCommandArgument
{
public function __construct(
public readonly string|null $name = null,
public readonly ?string $name = null,
) {
}
}
2 changes: 1 addition & 1 deletion src/Attribute/AsOption.php
Expand Up @@ -12,7 +12,7 @@ class AsOption extends AsCommandArgument
public function __construct(
?string $name = null,
public readonly string|array|null $shortcut = null,
public readonly int|null $mode = null,
public readonly ?int $mode = null,
public readonly string $description = '',
public readonly array $suggestedValues = [],
) {
Expand Down
2 changes: 1 addition & 1 deletion src/Attribute/AsTask.php
Expand Up @@ -11,7 +11,7 @@ class AsTask
*/
public function __construct(
public string $name = '',
public string|null $namespace = null,
public ?string $namespace = null,
public string $description = '',
public array $aliases = [],
public array $onSignals = [],
Expand Down
4 changes: 2 additions & 2 deletions src/Context.php
Expand Up @@ -17,7 +17,7 @@ public function __construct(
?string $currentDirectory = null,
public readonly bool $tty = false,
public readonly bool $pty = true,
public readonly float|null $timeout = null,
public readonly ?float $timeout = null,
public readonly bool $quiet = false,
public readonly bool $allowFailure = false,
public readonly bool $notify = false,
Expand Down Expand Up @@ -132,7 +132,7 @@ public function withPty(bool $pty = true): self
);
}

public function withTimeout(float|null $timeout): self
public function withTimeout(?float $timeout): self
{
return new self(
$this->data,
Expand Down
8 changes: 5 additions & 3 deletions src/ContextRegistry.php
Expand Up @@ -2,6 +2,8 @@

namespace Castor;

use Symfony\Component\PropertyAccess\PropertyAccess;

/** @internal */
class ContextRegistry
{
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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:

$context = ['foo' => 'bar', 'nested' => ['a' => 'b']];
$foo = variable('bar');
$a = variable('nested['a']);

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 [nested][a]

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.

$a = variable('nested')['a'] ?? null

And worse if null is not the default value

$a = variable('nested', 'another')['a'] ?? 'another'

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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/SectionOutput.php
Expand Up @@ -14,7 +14,7 @@ class SectionOutput

private OutputInterface|ConsoleSectionOutput $consoleOutput;

private ConsoleOutput|null $mainOutput;
private ?ConsoleOutput $mainOutput;

/** @var \SplObjectStorage<Process, SectionDetails> */
private \SplObjectStorage $sections;
Expand Down
Expand Up @@ -2,3 +2,4 @@ context name: dynamic
Production? no
verbosity: 1
context: baz
nested key: N/A
Expand Up @@ -2,3 +2,4 @@ context name: my_default
Production? no
verbosity: 2
context: bar
nested key: nested_value
Expand Up @@ -2,3 +2,4 @@ context name: path
Production? yes
verbosity: 1
context: bar
nested key: N/A
Expand Up @@ -2,3 +2,4 @@ context name: production
Production? yes
verbosity: 1
context: bar
nested key: nested_value
Expand Up @@ -2,3 +2,4 @@ context name: run
Production? no
verbosity: 1
context: no defined
nested key: N/A
1 change: 1 addition & 0 deletions tests/Examples/Generated/ContextContextTest.php.output.txt
Expand Up @@ -2,3 +2,4 @@ context name: my_default
Production? no
verbosity: 1
context: bar
nested key: nested_value
Expand Up @@ -2,4 +2,5 @@ context name: dynamic
Production? no
verbosity: -1
context: bar
nested key: N/A
bar